diff --git a/.github/actions/common_setup/action.yml b/.github/actions/common_setup/action.yml index b9299c64e72a..de7215d2103a 100644 --- a/.github/actions/common_setup/action.yml +++ b/.github/actions/common_setup/action.yml @@ -28,6 +28,16 @@ runs: run: | # to remove every leftovers sudo rm -fr "$TEMP_PATH" && mkdir -p "$TEMP_PATH" + - name: Setup zram + shell: bash + run: | + sudo modprobe zram + MemTotal=$(grep -Po "(?<=MemTotal:)\s+\d+" /proc/meminfo) # KiB + Percent=200 + ZRAM_SIZE=$(($MemTotal / 1024 / 1024 * $Percent / 100)) # Convert to GiB + .github/retry.sh 30 2 sudo zramctl --size ${ZRAM_SIZE}GiB --algorithm zstd /dev/zram0 + sudo mkswap /dev/zram0 && sudo swapon -p 100 /dev/zram0 + sudo sysctl vm.swappiness=200 - name: Tune vm.mmap_rnd_bits for sanitizers shell: bash run: | diff --git a/.github/workflows/release_branches.yml b/.github/workflows/release_branches.yml index bbee341c6324..39ed2f240495 100644 --- a/.github/workflows/release_branches.yml +++ b/.github/workflows/release_branches.yml @@ -190,7 +190,7 @@ jobs: - name: Builds report run: | cd "$GITHUB_WORKSPACE/tests/ci" - python3 ./build_report_check.py --reports package_release package_aarch64 package_asan package_msan package_ubsan package_tsan package_debug binary_darwin binary_darwin_aarch64 + python3 ./build_report_check.py --reports package_release package_aarch64 package_asan package_msan package_ubsan package_tsan package_debug - name: Set status # NOTE(vnemkov): generate and upload the report even if previous step failed if: success() || failure() @@ -547,7 +547,7 @@ jobs: - RegressionTestsRelease - RegressionTestsAarch64 - SignRelease - runs-on: [self-hosted, altinity-on-demand, altinity-type-cax11, altinity-image-arm-system-ubuntu-22.04] + runs-on: [self-hosted, altinity-on-demand, altinity-type-cax11, altinity-image-arm-snapshot-22.04-arm, altinity-startup-snapshot, altinity-setup-none] steps: - name: Check out repository code uses: Altinity/checkout@19599efdf36c4f3f30eb55d5bb388896faea69f6 diff --git a/.github/workflows/reusable_build.yml b/.github/workflows/reusable_build.yml index 428f9c0842f1..c64d0aaec500 100644 --- a/.github/workflows/reusable_build.yml +++ b/.github/workflows/reusable_build.yml @@ -4,7 +4,6 @@ env: # Force the stdout and stderr streams to be unbuffered PYTHONUNBUFFERED: 1 - CLICKHOUSE_STABLE_VERSION_SUFFIX: altinityedge AWS_ACCESS_KEY_ID: ${{ secrets.AWS_ACCESS_KEY_ID }} AWS_DEFAULT_REGION: ${{ secrets.AWS_DEFAULT_REGION }} AWS_SECRET_ACCESS_KEY: ${{ secrets.AWS_SECRET_ACCESS_KEY }} @@ -54,7 +53,7 @@ jobs: if: ${{ contains(fromJson(inputs.data).jobs_data.jobs_to_do, inputs.build_name) || inputs.force }} env: GITHUB_JOB_OVERRIDDEN: Build-${{inputs.build_name}} - runs-on: [self-hosted, altinity-setup-builder, altinity-type-ccx53, altinity-on-demand, altinity-in-ash, altinity-image-x86-system-ubuntu-22.04] + runs-on: [self-hosted, altinity-type-ccx53, altinity-on-demand, altinity-image-x86-snapshot-22.04-amd, altinity-startup-snapshot, altinity-setup-none] steps: - name: Check out repository code uses: Altinity/checkout@19599efdf36c4f3f30eb55d5bb388896faea69f6 diff --git a/cmake/autogenerated_versions.txt b/cmake/autogenerated_versions.txt index 2bb36820d1c0..1706e8010bb7 100644 --- a/cmake/autogenerated_versions.txt +++ b/cmake/autogenerated_versions.txt @@ -2,16 +2,18 @@ # NOTE: VERSION_REVISION has nothing common with DBMS_TCP_PROTOCOL_VERSION, # only DBMS_TCP_PROTOCOL_VERSION should be incremented on protocol changes. -SET(VERSION_REVISION 54495) +SET(VERSION_REVISION 54496) SET(VERSION_MAJOR 24) SET(VERSION_MINOR 8) -SET(VERSION_PATCH 7) -SET(VERSION_GITHASH ddb8c2197719757fcc7ecee79079b00ebd8a7487) +SET(VERSION_PATCH 8) +SET(VERSION_GITHASH e28553d4f2ba78643f9ef47b698954a2c54e6bcc) -SET(VERSION_TWEAK 43) +#1000 for altinitystable candidates +#2000 for altinityedge candidates +SET(VERSION_TWEAK 182000) SET(VERSION_FLAVOUR altinityedge) -SET(VERSION_DESCRIBE v24.8.7.43.altinityedge) -SET(VERSION_STRING 24.8.7.43) +SET(VERSION_DESCRIBE v24.8.8.182000.altinityedge) +SET(VERSION_STRING 24.8.8.182000) # end of autochange diff --git a/cmake/version.cmake b/cmake/version.cmake index 06fb783b88f2..b008c989c0b0 100644 --- a/cmake/version.cmake +++ b/cmake/version.cmake @@ -3,9 +3,10 @@ include(${PROJECT_SOURCE_DIR}/cmake/autogenerated_versions.txt) set(VERSION_EXTRA "" CACHE STRING "") set(VERSION_TWEAK "" CACHE STRING "") -if (VERSION_TWEAK) - string(CONCAT VERSION_STRING ${VERSION_STRING} "." ${VERSION_TWEAK}) -endif () +# NOTE(vnemkov): we rely on VERSION_TWEAK portion to be already present in VERSION_STRING +# if (VERSION_TWEAK) +# string(CONCAT VERSION_STRING ${VERSION_STRING} "." ${VERSION_TWEAK}) +# endif () if (VERSION_EXTRA) string(CONCAT VERSION_STRING ${VERSION_STRING} "." ${VERSION_EXTRA}) diff --git a/docker/test/stateful/run.sh b/docker/test/stateful/run.sh index c072eeb0fa8b..af578423b043 100755 --- a/docker/test/stateful/run.sh +++ b/docker/test/stateful/run.sh @@ -22,8 +22,7 @@ source /utils.lib # install test configs /usr/share/clickhouse-test/config/install.sh -azurite-blob --blobHost 0.0.0.0 --blobPort 10000 --silent --inMemoryPersistence & - +azurite-blob --blobHost 0.0.0.0 --blobPort 10000 --debug /azurite_log & ./setup_minio.sh stateful ./mc admin trace clickminio > /test_output/minio.log & MC_ADMIN_PID=$! diff --git a/docker/test/stateless/Dockerfile b/docker/test/stateless/Dockerfile index 71a68df7e0b7..7e57ad900a49 100644 --- a/docker/test/stateless/Dockerfile +++ b/docker/test/stateless/Dockerfile @@ -87,7 +87,7 @@ ENV MINIO_ROOT_PASSWORD="clickhouse" ENV EXPORT_S3_STORAGE_POLICIES=1 ENV CLICKHOUSE_GRPC_CLIENT="/usr/share/clickhouse-utils/grpc-client/clickhouse-grpc-client.py" -RUN npm install -g azurite@3.30.0 \ +RUN npm install -g azurite@^3.33.0 \ && npm install -g tslib && npm install -g node COPY run.sh / diff --git a/docker/test/stateless/run.sh b/docker/test/stateless/run.sh index ad0cd321cc55..13d79243713b 100755 --- a/docker/test/stateless/run.sh +++ b/docker/test/stateless/run.sh @@ -50,6 +50,12 @@ source /utils.lib # install test configs /usr/share/clickhouse-test/config/install.sh +if [[ -n "$USE_DATABASE_REPLICATED" ]] && [[ "$USE_DATABASE_REPLICATED" -eq 1 ]]; then + echo "Azure is disabled" +else + azurite-blob --blobHost 0.0.0.0 --blobPort 10000 --debug /azurite_log & +fi + ./setup_minio.sh stateless ./setup_hdfs_minicluster.sh diff --git a/docker/test/stress/run.sh b/docker/test/stress/run.sh index b21114e456f4..48348aa131ca 100644 --- a/docker/test/stress/run.sh +++ b/docker/test/stress/run.sh @@ -55,6 +55,7 @@ export ZOOKEEPER_FAULT_INJECTION=1 # available for dump via clickhouse-local configure +azurite-blob --blobHost 0.0.0.0 --blobPort 10000 --debug /azurite_log & ./setup_minio.sh stateless # to have a proper environment config_logs_export_cluster /etc/clickhouse-server/config.d/system_logs_export.yaml diff --git a/src/Common/FailPoint.cpp b/src/Common/FailPoint.cpp index bc4f2cb43d21..24ac4d8378d8 100644 --- a/src/Common/FailPoint.cpp +++ b/src/Common/FailPoint.cpp @@ -64,6 +64,10 @@ static struct InitFiu REGULAR(lazy_pipe_fds_fail_close) \ PAUSEABLE(infinite_sleep) \ PAUSEABLE(stop_moving_part_before_swap_with_active) \ + REGULAR(slowdown_index_analysis) \ + REGULAR(replicated_merge_tree_all_replicas_stale) \ + REGULAR(zero_copy_lock_zk_fail_before_op) \ + REGULAR(zero_copy_lock_zk_fail_after_op) \ namespace FailPoints diff --git a/src/Functions/DateTimeTransforms.h b/src/Functions/DateTimeTransforms.h index dfb4b76e5e21..f52c83fa8abb 100644 --- a/src/Functions/DateTimeTransforms.h +++ b/src/Functions/DateTimeTransforms.h @@ -63,6 +63,7 @@ constexpr time_t MAX_DATETIME_DAY_NUM = 49710; // 2106-02-07 /// This factor transformation will say that the function is monotone everywhere. struct ZeroTransform { + static constexpr auto name = "Zero"; static UInt16 execute(Int64, const DateLUTImpl &) { return 0; } static UInt16 execute(UInt32, const DateLUTImpl &) { return 0; } static UInt16 execute(Int32, const DateLUTImpl &) { return 0; } diff --git a/src/Functions/IFunctionCustomWeek.h b/src/Functions/IFunctionCustomWeek.h index ba0baa358199..c6f1aebea881 100644 --- a/src/Functions/IFunctionCustomWeek.h +++ b/src/Functions/IFunctionCustomWeek.h @@ -55,13 +55,26 @@ class IFunctionCustomWeek : public IFunction ? is_monotonic : is_not_monotonic; } - else + + if (checkAndGetDataType(&type)) { - return Transform::FactorTransform::execute(UInt32(left.safeGet()), date_lut) - == Transform::FactorTransform::execute(UInt32(right.safeGet()), date_lut) + + const auto & left_date_time = left.safeGet(); + TransformDateTime64 transformer_left(left_date_time.getScale()); + + const auto & right_date_time = right.safeGet(); + TransformDateTime64 transformer_right(right_date_time.getScale()); + + return transformer_left.execute(left_date_time.getValue(), date_lut) + == transformer_right.execute(right_date_time.getValue(), date_lut) ? is_monotonic : is_not_monotonic; } + + return Transform::FactorTransform::execute(UInt32(left.safeGet()), date_lut) + == Transform::FactorTransform::execute(UInt32(right.safeGet()), date_lut) + ? is_monotonic + : is_not_monotonic; } protected: diff --git a/src/Parsers/ASTAlterQuery.cpp b/src/Parsers/ASTAlterQuery.cpp index 58eeb7c4cbf3..8bcb6fdc2fc7 100644 --- a/src/Parsers/ASTAlterQuery.cpp +++ b/src/Parsers/ASTAlterQuery.cpp @@ -70,8 +70,12 @@ ASTPtr ASTAlterCommand::clone() const void ASTAlterCommand::formatImpl(const FormatSettings & settings, FormatState & state, FormatStateStacked frame) const { + scope_guard closing_bracket_guard; if (format_alter_commands_with_parentheses) + { settings.ostr << "("; + closing_bracket_guard = make_scope_guard(std::function([&settings]() { settings.ostr << ")"; })); + } if (type == ASTAlterCommand::ADD_COLUMN) { @@ -498,9 +502,6 @@ void ASTAlterCommand::formatImpl(const FormatSettings & settings, FormatState & } else throw Exception(ErrorCodes::UNEXPECTED_AST_STRUCTURE, "Unexpected type of ALTER"); - - if (format_alter_commands_with_parentheses) - settings.ostr << ")"; } void ASTAlterCommand::forEachPointerToChild(std::function f) diff --git a/src/Parsers/ExpressionElementParsers.cpp b/src/Parsers/ExpressionElementParsers.cpp index dd22b80b1cb8..6700845b2fd7 100644 --- a/src/Parsers/ExpressionElementParsers.cpp +++ b/src/Parsers/ExpressionElementParsers.cpp @@ -140,6 +140,9 @@ bool ParserSubquery::parseImpl(Pos & pos, ASTPtr & node, Expected & expected) const ASTPtr & explained_ast = explain_query.getExplainedQuery(); if (explained_ast) { + if (!explained_ast->as()) + throw Exception(ErrorCodes::BAD_ARGUMENTS, "EXPLAIN inside subquery supports only SELECT queries"); + auto view_explain = makeASTFunction("viewExplain", std::make_shared(kind_str), std::make_shared(settings_str), diff --git a/src/Parsers/IParser.cpp b/src/Parsers/IParser.cpp index 9d7a0a65b343..6c6928a88029 100644 --- a/src/Parsers/IParser.cpp +++ b/src/Parsers/IParser.cpp @@ -53,7 +53,12 @@ void Expected::highlight(HighlightedRange range) /// for each highlight x and the next one y: x.end <= y.begin, thus preventing any overlap. if (it != highlights.begin()) - it = std::prev(it); + { + auto prev_it = std::prev(it); + + if (range.begin < prev_it->end) + it = prev_it; + } while (it != highlights.end() && range.begin < it->end) { diff --git a/src/Storages/MergeTree/IMergeTreeDataPart.cpp b/src/Storages/MergeTree/IMergeTreeDataPart.cpp index ceec13fc4a4b..faa5e3a6f1f6 100644 --- a/src/Storages/MergeTree/IMergeTreeDataPart.cpp +++ b/src/Storages/MergeTree/IMergeTreeDataPart.cpp @@ -524,6 +524,14 @@ SerializationPtr IMergeTreeDataPart::tryGetSerialization(const String & column_n return it == serializations.end() ? nullptr : it->second; } +bool IMergeTreeDataPart::isMovingPart() const +{ + fs::path part_directory_path = getDataPartStorage().getRelativePath(); + if (part_directory_path.filename().empty()) + part_directory_path = part_directory_path.parent_path(); + return part_directory_path.parent_path().filename() == "moving"; +} + void IMergeTreeDataPart::removeIfNeeded() { assert(assertHasValidVersionMetadata()); @@ -548,10 +556,7 @@ void IMergeTreeDataPart::removeIfNeeded() throw Exception(ErrorCodes::LOGICAL_ERROR, "relative_path {} of part {} is invalid or not set", getDataPartStorage().getPartDirectory(), name); - fs::path part_directory_path = getDataPartStorage().getRelativePath(); - if (part_directory_path.filename().empty()) - part_directory_path = part_directory_path.parent_path(); - bool is_moving_part = part_directory_path.parent_path().filename() == "moving"; + bool is_moving_part = isMovingPart(); if (!startsWith(file_name, "tmp") && !endsWith(file_name, ".tmp_proj") && !is_moving_part) { LOG_ERROR( diff --git a/src/Storages/MergeTree/IMergeTreeDataPart.h b/src/Storages/MergeTree/IMergeTreeDataPart.h index 85ef0472ce7e..07050697f4de 100644 --- a/src/Storages/MergeTree/IMergeTreeDataPart.h +++ b/src/Storages/MergeTree/IMergeTreeDataPart.h @@ -429,6 +429,9 @@ class IMergeTreeDataPart : public std::enable_shared_from_thisgetArgumentTypes()[0]; + auto in_argument_type = getArgumentTypeOfMonotonicFunction(*functions[0]); if (canBeSafelyCasted(result_type, in_argument_type)) { result_column = castColumnAccurate({result_column, result_type, ""}, in_argument_type); @@ -974,7 +976,7 @@ bool applyFunctionChainToColumn( if (func->getArgumentTypes().empty()) return false; - auto argument_type = func->getArgumentTypes()[0]; + auto argument_type = getArgumentTypeOfMonotonicFunction(*func); if (!canBeSafelyCasted(result_type, argument_type)) return false; @@ -1384,6 +1386,18 @@ class FunctionWithOptionalConstArg : public IFunctionBase Kind kind = Kind::NO_CONST; }; +DataTypePtr getArgumentTypeOfMonotonicFunction(const IFunctionBase & func) +{ + const auto & arg_types = func.getArgumentTypes(); + if (const auto * func_ptr = typeid_cast(&func)) + { + if (func_ptr->getKind() == FunctionWithOptionalConstArg::Kind::LEFT_CONST) + return arg_types.at(1); + } + + return arg_types.at(0); +} + bool KeyCondition::isKeyPossiblyWrappedByMonotonicFunctions( const RPNBuilderTreeNode & node, diff --git a/src/Storages/MergeTree/MergeTreeData.cpp b/src/Storages/MergeTree/MergeTreeData.cpp index a4fb314d5d9a..7670c88b94f5 100644 --- a/src/Storages/MergeTree/MergeTreeData.cpp +++ b/src/Storages/MergeTree/MergeTreeData.cpp @@ -7998,33 +7998,49 @@ MovePartsOutcome MergeTreeData::moveParts(const CurrentlyMovingPartsTaggerPtr & /// replica will actually move the part from disk to some /// zero-copy storage other replicas will just fetch /// metainformation. - if (auto lock = tryCreateZeroCopyExclusiveLock(moving_part.part->name, disk); lock) + auto lock = tryCreateZeroCopyExclusiveLock(moving_part.part->name, disk); + if (!lock) { + /// Move will be retried but with backoff. + LOG_DEBUG( + log, + "Move of part {} postponed, because zero copy mode enabled and zero-copy lock was not acquired", + moving_part.part->name); + result = MovePartsOutcome::MoveWasPostponedBecauseOfZeroCopy; + break; + } + + if (lock->isLocked()) + { + cloned_part = parts_mover.clonePart(moving_part, read_settings, write_settings); + /// Cloning part can take a long time. + /// Recheck if the lock (and keeper session expirity) is OK if (lock->isLocked()) { - cloned_part = parts_mover.clonePart(moving_part, read_settings, write_settings); parts_mover.swapClonedPart(cloned_part); + break; /// Successfully moved + } + else + { + LOG_DEBUG( + log, + "Move of part {} postponed, because zero copy mode enabled and zero-copy lock was lost during cloning the part", + moving_part.part->name); + result = MovePartsOutcome::MoveWasPostponedBecauseOfZeroCopy; break; } - else if (wait_for_move_if_zero_copy) + } + if (wait_for_move_if_zero_copy) + { + LOG_DEBUG(log, "Other replica is working on move of {}, will wait until lock disappear", moving_part.part->name); + /// Wait and checks not only for timeout but also for shutdown and so on. + while (!waitZeroCopyLockToDisappear(*lock, 3000)) { - LOG_DEBUG(log, "Other replica is working on move of {}, will wait until lock disappear", moving_part.part->name); - /// Wait and checks not only for timeout but also for shutdown and so on. - while (!waitZeroCopyLockToDisappear(*lock, 3000)) - { - LOG_DEBUG(log, "Waiting until some replica will move {} and zero copy lock disappear", moving_part.part->name); - } + LOG_DEBUG(log, "Waiting until some replica will move {} and zero copy lock disappear", moving_part.part->name); } - else - break; } else - { - /// Move will be retried but with backoff. - LOG_DEBUG(log, "Move of part {} postponed, because zero copy mode enabled and someone other moving this part right now", moving_part.part->name); - result = MovePartsOutcome::MoveWasPostponedBecauseOfZeroCopy; break; - } } } else /// Ordinary move as it should be diff --git a/src/Storages/MergeTree/MergeTreePartsMover.cpp b/src/Storages/MergeTree/MergeTreePartsMover.cpp index d81300da7386..528e2a9d55ae 100644 --- a/src/Storages/MergeTree/MergeTreePartsMover.cpp +++ b/src/Storages/MergeTree/MergeTreePartsMover.cpp @@ -255,6 +255,7 @@ MergeTreePartsMover::TemporaryClonedPart MergeTreePartsMover::clonePart(const Me disk->createDirectories(path_to_clone); + /// TODO: Make it possible to fetch only zero-copy part without fallback to fetching a full-copy one auto zero_copy_part = data->tryToFetchIfShared(*part, disk, fs::path(path_to_clone) / part->name); if (zero_copy_part) @@ -297,6 +298,28 @@ MergeTreePartsMover::TemporaryClonedPart MergeTreePartsMover::clonePart(const Me return cloned_part; } +void MergeTreePartsMover::renameClonedPart(IMergeTreeDataPart & part) const +try +{ + part.is_temp = false; + /// Mark it DeleteOnDestroy to ensure deleting in destructor + /// if something goes wrong before swapping + part.setState(MergeTreeDataPartState::DeleteOnDestroy); + /// Don't remove new directory but throw an error because it may contain part which is currently in use. + part.renameTo(part.name, /* remove_new_dir_if_exists */ false); +} +catch (...) +{ + /// Check if part was renamed or not + /// `renameTo()` does not provide strong exception guarantee in case of an exception + if (part.isMovingPart()) + { + /// Restore its temporary state + part.is_temp = true; + part.setState(MergeTreeDataPartState::Temporary); + } + throw; +} void MergeTreePartsMover::swapClonedPart(TemporaryClonedPart & cloned_part) const { @@ -323,12 +346,23 @@ void MergeTreePartsMover::swapClonedPart(TemporaryClonedPart & cloned_part) cons return; } - cloned_part.part->is_temp = false; - - /// Don't remove new directory but throw an error because it may contain part which is currently in use. - cloned_part.part->renameTo(active_part->name, false); - - /// TODO what happen if server goes down here? + /// It is safe to acquire zero-copy lock for the temporary part here + /// because no one can fetch it until it is *swapped*. + /// + /// Set ASK_KEEPER to try to unlock it in destructor if something goes wrong before *renaming* + /// If unlocking is failed we will not get a stuck part in moving directory + /// because it will be renamed to delete_tmp_ beforehand and cleaned up later. + /// Worst outcomes: trash in object storage and/or orphaned shared zero-copy lock. It is acceptable. + /// See DataPartStorageOnDiskBase::remove(). + cloned_part.part->remove_tmp_policy = IMergeTreeDataPart::BlobsRemovalPolicyForTemporaryParts::ASK_KEEPER; + data->lockSharedData(*cloned_part.part, /* replace_existing_lock = */ true); + + renameClonedPart(*cloned_part.part); + + /// If server goes down here we will get two copy of the part with the same name on different disks. + /// And on the next ClickHouse startup during loading parts the first copy (in the order of defining disks + /// in the storage policy) will be loaded as Active, the second one will be loaded as Outdated and removed as duplicate. + /// See MergeTreeData::loadDataParts(). data->swapActivePart(cloned_part.part, part_lock); LOG_TRACE(log, "Part {} was moved to {}", cloned_part.part->name, cloned_part.part->getDataPartStorage().getFullPath()); diff --git a/src/Storages/MergeTree/MergeTreePartsMover.h b/src/Storages/MergeTree/MergeTreePartsMover.h index 3cf270946d86..7a6583008c0d 100644 --- a/src/Storages/MergeTree/MergeTreePartsMover.h +++ b/src/Storages/MergeTree/MergeTreePartsMover.h @@ -75,6 +75,9 @@ class MergeTreePartsMover /// merge or mutation. void swapClonedPart(TemporaryClonedPart & cloned_part) const; + /// Rename cloned part from `moving/` directory to the actual part storage + void renameClonedPart(IMergeTreeDataPart & part) const; + /// Can stop background moves and moves from queries ActionBlocker moves_blocker; diff --git a/src/Storages/MergeTree/ReplicatedMergeTreeAttachThread.cpp b/src/Storages/MergeTree/ReplicatedMergeTreeAttachThread.cpp index 6e22a3515bc5..67570d783667 100644 --- a/src/Storages/MergeTree/ReplicatedMergeTreeAttachThread.cpp +++ b/src/Storages/MergeTree/ReplicatedMergeTreeAttachThread.cpp @@ -15,6 +15,7 @@ namespace ErrorCodes { extern const int SUPPORT_IS_DISABLED; extern const int REPLICA_STATUS_CHANGED; + extern const int LOGICAL_ERROR; } ReplicatedMergeTreeAttachThread::ReplicatedMergeTreeAttachThread(StorageReplicatedMergeTree & storage_) @@ -117,6 +118,67 @@ void ReplicatedMergeTreeAttachThread::checkHasReplicaMetadataInZooKeeper(const z } } +Int32 ReplicatedMergeTreeAttachThread::fixReplicaMetadataVersionIfNeeded(zkutil::ZooKeeperPtr zookeeper) +{ + const String & zookeeper_path = storage.zookeeper_path; + const String & replica_path = storage.replica_path; + const bool replica_readonly = storage.is_readonly; + + for (size_t i = 0; i != 2; ++i) + { + String replica_metadata_version_str; + const bool replica_metadata_version_exists = zookeeper->tryGet(replica_path + "/metadata_version", replica_metadata_version_str); + if (!replica_metadata_version_exists) + return -1; + + const Int32 metadata_version = parse(replica_metadata_version_str); + + if (metadata_version != 0 || replica_readonly) + { + /// No need to fix anything + return metadata_version; + } + + Coordination::Stat stat; + zookeeper->get(fs::path(zookeeper_path) / "metadata", &stat); + if (stat.version == 0) + { + /// No need to fix anything + return metadata_version; + } + + ReplicatedMergeTreeQueue & queue = storage.queue; + queue.pullLogsToQueue(zookeeper); + if (queue.getStatus().metadata_alters_in_queue != 0) + { + LOG_DEBUG(log, "No need to update metadata_version as there are ALTER_METADATA entries in the queue"); + return metadata_version; + } + + const Coordination::Requests ops = { + zkutil::makeSetRequest(fs::path(replica_path) / "metadata_version", std::to_string(stat.version), 0), + zkutil::makeCheckRequest(fs::path(zookeeper_path) / "metadata", stat.version), + }; + Coordination::Responses ops_responses; + const auto code = zookeeper->tryMulti(ops, ops_responses); + if (code == Coordination::Error::ZOK) + { + LOG_DEBUG(log, "Successfully set metadata_version to {}", stat.version); + return stat.version; + } + if (code != Coordination::Error::ZBADVERSION) + { + throw zkutil::KeeperException(code); + } + } + + /// Second attempt is only possible if metadata_version != 0 or metadata.version changed during the first attempt. + /// If metadata_version != 0, on second attempt we will return the new metadata_version. + /// If metadata.version changed, on second attempt we will either get metadata_version != 0 and return the new metadata_version or we will get metadata_alters_in_queue != 0 and return 0. + /// Either way, on second attempt this method should return. + throw Exception(ErrorCodes::LOGICAL_ERROR, "Failed to fix replica metadata_version in ZooKeeper after two attempts"); +} + void ReplicatedMergeTreeAttachThread::runImpl() { storage.setZooKeeper(); @@ -160,11 +222,11 @@ void ReplicatedMergeTreeAttachThread::runImpl() /// Just in case it was not removed earlier due to connection loss zookeeper->tryRemove(replica_path + "/flags/force_restore_data"); - String replica_metadata_version; - const bool replica_metadata_version_exists = zookeeper->tryGet(replica_path + "/metadata_version", replica_metadata_version); + const Int32 replica_metadata_version = fixReplicaMetadataVersionIfNeeded(zookeeper); + const bool replica_metadata_version_exists = replica_metadata_version != -1; if (replica_metadata_version_exists) { - storage.setInMemoryMetadata(metadata_snapshot->withMetadataVersion(parse(replica_metadata_version))); + storage.setInMemoryMetadata(metadata_snapshot->withMetadataVersion(replica_metadata_version)); } else { diff --git a/src/Storages/MergeTree/ReplicatedMergeTreeAttachThread.h b/src/Storages/MergeTree/ReplicatedMergeTreeAttachThread.h index 250a5ed34d1c..bfc97442598a 100644 --- a/src/Storages/MergeTree/ReplicatedMergeTreeAttachThread.h +++ b/src/Storages/MergeTree/ReplicatedMergeTreeAttachThread.h @@ -48,6 +48,8 @@ class ReplicatedMergeTreeAttachThread void runImpl(); void finalizeInitialization(); + + Int32 fixReplicaMetadataVersionIfNeeded(zkutil::ZooKeeperPtr zookeeper); }; } diff --git a/src/Storages/MergeTree/ReplicatedMergeTreeQueue.cpp b/src/Storages/MergeTree/ReplicatedMergeTreeQueue.cpp index 627bda3f8bfe..9004f986c5e9 100644 --- a/src/Storages/MergeTree/ReplicatedMergeTreeQueue.cpp +++ b/src/Storages/MergeTree/ReplicatedMergeTreeQueue.cpp @@ -2128,6 +2128,7 @@ ReplicatedMergeTreeQueue::Status ReplicatedMergeTreeQueue::getStatus() const res.inserts_in_queue = 0; res.merges_in_queue = 0; res.part_mutations_in_queue = 0; + res.metadata_alters_in_queue = 0; res.queue_oldest_time = 0; res.inserts_oldest_time = 0; res.merges_oldest_time = 0; @@ -2170,6 +2171,11 @@ ReplicatedMergeTreeQueue::Status ReplicatedMergeTreeQueue::getStatus() const res.oldest_part_to_mutate_to = entry->new_part_name; } } + + if (entry->type == LogEntry::ALTER_METADATA) + { + ++res.metadata_alters_in_queue; + } } return res; diff --git a/src/Storages/MergeTree/ReplicatedMergeTreeQueue.h b/src/Storages/MergeTree/ReplicatedMergeTreeQueue.h index 89ef6240558a..2011d84eefe0 100644 --- a/src/Storages/MergeTree/ReplicatedMergeTreeQueue.h +++ b/src/Storages/MergeTree/ReplicatedMergeTreeQueue.h @@ -453,6 +453,7 @@ class ReplicatedMergeTreeQueue UInt32 inserts_in_queue; UInt32 merges_in_queue; UInt32 part_mutations_in_queue; + UInt32 metadata_alters_in_queue; UInt32 queue_oldest_time; UInt32 inserts_oldest_time; UInt32 merges_oldest_time; diff --git a/src/Storages/StorageDictionary.cpp b/src/Storages/StorageDictionary.cpp index b43cc4fa426f..07f4a8e98a0b 100644 --- a/src/Storages/StorageDictionary.cpp +++ b/src/Storages/StorageDictionary.cpp @@ -1,4 +1,5 @@ #include +#include #include #include #include @@ -169,7 +170,18 @@ Pipe StorageDictionary::read( { auto registered_dictionary_name = location == Location::SameDatabaseAndNameAsDictionary ? getStorageID().getInternalDictionaryName() : dictionary_name; auto dictionary = getContext()->getExternalDictionariesLoader().getDictionary(registered_dictionary_name, local_context); - local_context->checkAccess(AccessType::dictGet, dictionary->getDatabaseOrNoDatabaseTag(), dictionary->getDictionaryID().getTableName()); + + /** + * For backward compatibility reasons we require either SELECT or dictGet permission to read directly from the dictionary. + * If none of these conditions are met - we ask to grant a dictGet. + */ + bool has_dict_get = local_context->getAccess()->isGranted( + AccessType::dictGet, dictionary->getDatabaseOrNoDatabaseTag(), dictionary->getDictionaryID().getTableName()); + bool has_select = local_context->getAccess()->isGranted( + AccessType::SELECT, dictionary->getDatabaseOrNoDatabaseTag(), dictionary->getDictionaryID().getTableName()); + if (!has_dict_get && !has_select) + local_context->checkAccess(AccessType::dictGet, dictionary->getDatabaseOrNoDatabaseTag(), dictionary->getDictionaryID().getTableName()); + return dictionary->read(column_names, max_block_size, threads); } diff --git a/src/Storages/StorageReplicatedMergeTree.cpp b/src/Storages/StorageReplicatedMergeTree.cpp index c894818708dc..3823cea40258 100644 --- a/src/Storages/StorageReplicatedMergeTree.cpp +++ b/src/Storages/StorageReplicatedMergeTree.cpp @@ -147,6 +147,8 @@ namespace FailPoints extern const char replicated_queue_fail_next_entry[]; extern const char replicated_queue_unfail_entries[]; extern const char finish_set_quorum_failed_parts[]; + extern const char zero_copy_lock_zk_fail_before_op[]; + extern const char zero_copy_lock_zk_fail_after_op[]; } namespace ErrorCodes @@ -10333,6 +10335,10 @@ void StorageReplicatedMergeTree::createZeroCopyLockNode( Coordination::Requests ops; Coordination::Responses responses; getZeroCopyLockNodeCreateOps(zookeeper, zookeeper_node, ops, mode, replace_existing_lock, path_to_set_hardlinked_files, hardlinked_files); + + fiu_do_on(FailPoints::zero_copy_lock_zk_fail_before_op, { zookeeper->forceFailureBeforeOperation(); }); + fiu_do_on(FailPoints::zero_copy_lock_zk_fail_after_op, { zookeeper->forceFailureAfterOperation(); }); + auto error = zookeeper->tryMulti(ops, responses); if (error == Coordination::Error::ZOK) { diff --git a/tests/ci/build_check.py b/tests/ci/build_check.py index 17cf409293c7..384435311069 100644 --- a/tests/ci/build_check.py +++ b/tests/ci/build_check.py @@ -12,12 +12,13 @@ from ci_config import CI from env_helper import REPO_COPY, S3_BUILDS_BUCKET, TEMP_PATH, S3_ACCESS_KEY_ID, S3_SECRET_ACCESS_KEY from git_helper import Git -from pr_info import PRInfo +from pr_info import PRInfo, EventType from report import FAILURE, SUCCESS, JobReport, StatusType from stopwatch import Stopwatch from tee_popen import TeePopen from version_helper import ( ClickHouseVersion, + VersionType, get_version_from_repo, update_version_local, ) @@ -164,16 +165,31 @@ def main(): version = get_version_from_repo(git=Git(True)) logging.info("Got version from repo %s", version.string) - official_flag = pr_info.number == 0 + # official_flag = pr_info.number == 0 - version_type = "testing" - if is_release_pr(pr_info): - version_type = "stable" - official_flag = True + # version_type = "testing" + # if is_release_pr(pr_info): + # version_type = "stable" + # official_flag = True + + # NOTE(vnemkov): For Altinity Stable builds, version flavor + # (last part of version, like 'altinitystable') is obtained from tag. + # If there is no tag, then version is considered to be 'testing' + version_type = version._flavour = VersionType.TESTING + official_flag = True + + if pr_info.event_type == EventType.PUSH \ + and pr_info.ref.startswith('/ref/tags/'): + tag_name = pr_info.ref.removeprefix('/ref/tags/') + version_type = tag_name.split('.')[-1] + version._flavour = version_type + logging.info("Using version from tag: %s => %s", tag_name, version) + + # TODO(vnemkov): make sure tweak part is incremented by 1 each time we merge a PR update_version_local(version, version_type) - logging.info("Updated local files with version") + logging.info("Updated local files with version %s", version) logging.info("Build short name %s", build_name) diff --git a/tests/ci/ci.py b/tests/ci/ci.py index 27fa0b104c01..465820a7488f 100644 --- a/tests/ci/ci.py +++ b/tests/ci/ci.py @@ -388,7 +388,7 @@ def _pre_action(s3, job_name, batch, indata, pr_info): _get_ext_check_name(job_name), ) ClickHouseHelper().insert_events_into( - db="default", table="checks", events=prepared_events + db="gh-data", table="checks", events=prepared_events ) print(f"Pre action done. Report files [{reports_files}] have been downloaded") @@ -966,7 +966,7 @@ def _add_build_to_version_history( print(f"::notice ::Log Adding record to versions history: {data}") - ch_helper.insert_event_into(db="default", table="version_history", event=data) + ch_helper.insert_event_into(db="gh-data", table="version_history", event=data) def _run_test(job_name: str, run_command: str) -> int: @@ -1376,7 +1376,7 @@ def main() -> int: _get_ext_check_name(args.job_name), ) ClickHouseHelper().insert_events_into( - db="default", table="checks", events=prepared_events + db="gh-data", table="checks", events=prepared_events ) ### POST action: end diff --git a/tests/ci/ci_buddy.py b/tests/ci/ci_buddy.py index 164af72f4be7..ae64e2c44c21 100644 --- a/tests/ci/ci_buddy.py +++ b/tests/ci/ci_buddy.py @@ -104,7 +104,9 @@ def _get_webhooks(): return json_string def post(self, message: str, channels: List[str]) -> None: - print(f"Posting slack message, dry_run [{self.dry_run}]") + print(f"Would've posted slack message, dry_run [{self.dry_run}], message: {message}") + # NOTE(vnemkov): we don't use slack for CI/CD no need to post messages + return if self.dry_run: urls = [self.channels[Channels.DRY_RUN]] else: diff --git a/tests/ci/functional_test_check.py b/tests/ci/functional_test_check.py index 744ec0a715cf..0fd8ba4dd638 100644 --- a/tests/ci/functional_test_check.py +++ b/tests/ci/functional_test_check.py @@ -16,7 +16,6 @@ from docker_images_helper import DockerImage, get_docker_image, pull_image from download_release_packages import download_last_release from env_helper import REPO_COPY, REPORT_PATH, TEMP_PATH -from get_robot_token import get_parameter_from_ssm from pr_info import PRInfo from report import ( ERROR, @@ -52,8 +51,9 @@ def get_additional_envs( check_name: str, run_by_hash_num: int, run_by_hash_total: int ) -> List[str]: result = [] - azure_connection_string = get_parameter_from_ssm("azure_connection_string") - result.append(f"AZURE_CONNECTION_STRING='{azure_connection_string}'") + # TODO(vnemkov): put proper Azure connection string into SSM and re-enable this one + # azure_connection_string = get_parameter_from_ssm("azure_connection_string") + # result.append(f"AZURE_CONNECTION_STRING='{azure_connection_string}'") if "DatabaseReplicated" in check_name: result.append("USE_DATABASE_REPLICATED=1") if "DatabaseOrdinary" in check_name: @@ -67,9 +67,6 @@ def get_additional_envs( result.append("RANDOMIZE_OBJECT_KEY_TYPE=1") if "analyzer" in check_name: result.append("USE_OLD_ANALYZER=1") - if "azure" in check_name: - assert "USE_S3_STORAGE_FOR_MERGE_TREE=1" not in result - result.append("USE_AZURE_STORAGE_FOR_MERGE_TREE=1") if run_by_hash_total != 0: result.append(f"RUN_BY_HASH_NUM={run_by_hash_num}") diff --git a/tests/ci/git_helper.py b/tests/ci/git_helper.py index 4cbb1542eafc..ca924a378a45 100644 --- a/tests/ci/git_helper.py +++ b/tests/ci/git_helper.py @@ -11,6 +11,20 @@ logger = logging.getLogger(__name__) +class VersionType: + LTS = "lts" + NEW = "new" + PRESTABLE = "altinityedge" + STABLE = "altinitystable" + TESTING = "altinitytest" + + VALID = (NEW, TESTING, PRESTABLE, STABLE, LTS, + # NOTE (vnemkov): we don't use those directly, but it is used in unit-tests + "stable", + "prestable", + "testing", + ) + # ^ and $ match subline in `multiple\nlines` # \A and \Z match only start and end of the whole string # NOTE (vnemkov): support both upstream tag style: v22.x.y.z-lts and Altinity tag style: v22.x.y.z.altinitystable @@ -19,7 +33,7 @@ TAG_REGEXP = ( r"\Av\d{2}" # First two digits of major part r"([.][1-9]\d*){3}" # minor.patch.tweak parts - r"-(new|testing|prestable|stable|lts|altinitystable|altinityedge)\Z" # suffix with a version type + fr"[.-]({'|'.join(VersionType.VALID)})\Z" # suffix with a version type ) SHA_REGEXP = re.compile(r"\A([0-9]|[a-f]){40}\Z") diff --git a/tests/ci/pr_info.py b/tests/ci/pr_info.py index 09a8cb563a1f..f97ea05e14da 100644 --- a/tests/ci/pr_info.py +++ b/tests/ci/pr_info.py @@ -80,8 +80,12 @@ def get_pr_for_commit(sha, ref): ref, sha, ) - first_pr = our_prs[0] - return first_pr + if len(our_prs) != 0: + first_pr = our_prs[0] + return first_pr + else: + return None + except Exception as ex: logging.error( "Cannot fetch PR info from commit ref %s, sha %s, exception: %s", @@ -132,8 +136,9 @@ def __init__( ref = github_event.get("ref", "refs/heads/master") if ref and ref.startswith("refs/heads/"): ref = ref[11:] + self.ref = ref # type: str e.g. "refs/pull/509/merge" or "refs/tags/v24.3.12.76.altinitystable" # Default values - self.base_ref = "" # type: str + self.base_ref = github_event.get("base_ref","") # type: str self.base_name = "" # type: str self.head_ref = "" # type: str self.head_name = "" # type: str diff --git a/tests/ci/stress_check.py b/tests/ci/stress_check.py index 9f1f36a5db28..48a4e2f87798 100644 --- a/tests/ci/stress_check.py +++ b/tests/ci/stress_check.py @@ -13,7 +13,6 @@ from clickhouse_helper import CiLogsCredentials from docker_images_helper import DockerImage, get_docker_image, pull_image from env_helper import REPO_COPY, REPORT_PATH, TEMP_PATH -from get_robot_token import get_parameter_from_ssm from pr_info import PRInfo from report import ERROR, JobReport, TestResults, read_test_results from stopwatch import Stopwatch @@ -34,14 +33,13 @@ def format(self, record): def get_additional_envs(check_name: str) -> List[str]: result = [] - azure_connection_string = get_parameter_from_ssm("azure_connection_string") - result.append(f"AZURE_CONNECTION_STRING='{azure_connection_string}'") + # TODO(vnemkov): put proper Azure connection string into SSM and re-enable this one + # azure_connection_string = get_parameter_from_ssm("azure_connection_string") + # result.append(f"AZURE_CONNECTION_STRING='{azure_connection_string}'") # some cloud-specific features require feature flags enabled # so we need this ENV to be able to disable the randomization # of feature flags result.append("RANDOMIZE_KEEPER_FEATURE_FLAGS=1") - if "azure" in check_name: - result.append("USE_AZURE_STORAGE_FOR_MERGE_TREE=1") if "s3" in check_name: result.append("USE_S3_STORAGE_FOR_MERGE_TREE=1") diff --git a/tests/ci/test_version.py b/tests/ci/test_version.py index 37ab00996429..30f7dbc6a762 100644 --- a/tests/ci/test_version.py +++ b/tests/ci/test_version.py @@ -22,9 +22,10 @@ def test_version_arg(self): ("v31.1.1.2-testing", vh.get_version_from_string("31.1.1.2")), ("refs/tags/v31.1.1.2-testing", vh.get_version_from_string("31.1.1.2")), ) - for test_case in cases: - version = vh.version_arg(test_case[0]) - self.assertEqual(test_case[1], version) + for i, test_case in enumerate(cases): + with self.subTest(test_case, i=i): + version = vh.version_arg(test_case[0]) + self.assertEqual(test_case[1], version) error_cases = ( "0.0.0", "1.1.1.a", @@ -48,33 +49,40 @@ class TestCase: expected: CHV cases = ( + # TestCase( + # "v24.6.1.1-new", + # 15, + # "v24.4.1.2088-stable", + # 415, + # CHV(24, 5, 1, 54487, None, 415), + # ), + # TestCase( + # "v24.6.1.1-testing", + # 15, + # "v24.4.1.2088-stable", + # 415, + # CHV(24, 5, 1, 54487, None, 15), + # ), + # TestCase( + # "v24.6.1.1-stable", + # 15, + # "v24.4.1.2088-stable", + # 415, + # CHV(24, 5, 1, 54487, None, 15), + # ), + # TestCase( + # "v24.5.1.1-stable", + # 15, + # "v24.4.1.2088-stable", + # 415, + # CHV(24, 5, 1, 54487, None, 15), + # ), TestCase( - "v24.6.1.1-new", - 15, + "v24.5.1.100-stable", + 0, "v24.4.1.2088-stable", 415, - CHV(24, 5, 1, 54487, None, 415), - ), - TestCase( - "v24.6.1.1-testing", - 15, - "v24.4.1.2088-stable", - 415, - CHV(24, 5, 1, 54487, None, 15), - ), - TestCase( - "v24.6.1.1-stable", - 15, - "v24.4.1.2088-stable", - 415, - CHV(24, 5, 1, 54487, None, 15), - ), - TestCase( - "v24.5.1.1-stable", - 15, - "v24.4.1.2088-stable", - 415, - CHV(24, 5, 1, 54487, None, 15), + CHV(24, 5, 1, 54487, None, 100), ), ) git = Git(True) diff --git a/tests/ci/version_helper.py b/tests/ci/version_helper.py index 1c121637437b..33f914b7b092 100755 --- a/tests/ci/version_helper.py +++ b/tests/ci/version_helper.py @@ -4,7 +4,7 @@ from pathlib import Path from typing import Any, Dict, Iterable, List, Literal, Optional, Set, Tuple, Union -from git_helper import TWEAK, Git, get_tags, git_runner, removeprefix +from git_helper import TWEAK, Git, get_tags, git_runner, removeprefix, VersionType FILE_WITH_VERSION_PATH = "cmake/autogenerated_versions.txt" CHANGELOG_IN_PATH = "debian/changelog.in" @@ -29,6 +29,7 @@ SET(VERSION_MINOR {minor}) SET(VERSION_PATCH {patch}) SET(VERSION_GITHASH {githash}) +SET(VERSION_TWEAK {tweak}) SET(VERSION_DESCRIBE {describe}) SET(VERSION_STRING {string}) # end of autochange @@ -251,21 +252,8 @@ def __repr__(self): ClickHouseVersions = List[ClickHouseVersion] -class VersionType: - LTS = "lts" - NEW = "new" - PRESTABLE = "prestable" - STABLE = "altinitystable" - TESTING = "altinityedge" - VALID = (NEW, TESTING, PRESTABLE, STABLE, LTS, - # NOTE (vnemkov): we don't use those directly, but it is used in unit-tests - "stable", - "testing" - ) - - def validate_version(version: str) -> None: - # NOTE(vnemkov): minor but imporant fixes, so versions with 'flavour' are threated as valid (e.g. 22.8.8.4.altinitystable) + # NOTE(vnemkov): minor but important fixes, so versions with 'flavour' are threated as valid (e.g. 22.8.8.4.altinitystable) parts = version.split(".") if len(parts) < 4: raise ValueError(f"{version} does not contain 4 parts") @@ -314,17 +302,32 @@ def get_version_from_repo( tweak=versions.get("tweak", versions["revision"]), flavour=versions.get("flavour", None) ) - # Since 24.5 we have tags like v24.6.1.1-new, and we must check if the release - # branch already has it's own commit. It's necessary for a proper tweak version + + # if this commit is tagged, use tag's version instead of something stored in cmake if git is not None and git.latest_tag: version_from_tag = get_version_from_tag(git.latest_tag) - if ( - version_from_tag.description == VersionType.NEW - and cmake_version < version_from_tag - ): - # We are in a new release branch without existing release. - # We should change the tweak version to a `tweak_to_new` - cmake_version.tweak = git.tweak_to_new + logging.debug(f'Git latest tag: {git.latest_tag} ({git.commits_since_latest} commits ago)\n' + f'"new" tag: {git.new_tag} ({git.commits_since_new})\n' + f'current commit: {git.sha}\n' + f'current brach: {git.branch}' + ) + if git.commits_since_latest == 0: + # Tag has a priority over the version written in CMake. + # Version must match (except tweak, flavour, description, etc.) to avoid accidental mess. + if not (version_from_tag.major == cmake_version.major \ + and version_from_tag.minor == cmake_version.minor \ + and version_from_tag.patch == cmake_version.patch): + raise RuntimeError(f"Version generated from tag ({version_from_tag}) should have same major, minor, and patch values as version generated from cmake ({cmake_version})") + + # Don't need to reset version completely, mostly because revision part is not set in tag, but must be preserved + logging.debug(f"Resetting TWEAK and FLAVOUR of version from cmake {cmake_version} to values from tag: {version_from_tag.tweak}.{version_from_tag._flavour}") + cmake_version._flavour = version_from_tag._flavour + cmake_version.tweak = version_from_tag.tweak + else: + # We've had some number of commits since the latest tag. + logging.debug(f"Bumping the TWEAK of version from cmake {cmake_version} by {git.commits_since_latest}") + cmake_version.tweak = cmake_version.tweak + git.commits_since_latest + return cmake_version @@ -347,9 +350,16 @@ def get_version_from_string( def get_version_from_tag(tag: str) -> ClickHouseVersion: Git.check_tag(tag) - tag, description = tag[1:].split("-", 1) - version = get_version_from_string(tag) - version.with_description(description) + tag = tag[1:] # strip initial 'v' + if '-' in tag: + # Upstream tags with dash + tag, description = tag.split("-", 1) + version = get_version_from_string(tag) + version.with_description(description) + else: + # Altinity's tags, with dots as separators between parts (handled properly down the road) + version = get_version_from_string(tag) + return version diff --git a/tests/config/config.d/azure_storage_conf.xml b/tests/config/config.d/azure_storage_conf.xml index f24b62b87b10..412d40111a70 100644 --- a/tests/config/config.d/azure_storage_conf.xml +++ b/tests/config/config.d/azure_storage_conf.xml @@ -4,10 +4,13 @@ object_storage azure + http://localhost:10000/devstoreaccount1 + cont false + + devstoreaccount1 + Eby8vdM02xNOcqFlqUwJPLlmEtlCDXJ1OUzFT50uSRZ6IFsuFq2UVErCz4I6tq/K1SZFPTOtr/KBHBeksoGMGw== 33554432 - openbucketforpublicci - cache diff --git a/tests/config/config.d/azure_storage_policy_by_default.xml b/tests/config/config.d/azure_storage_policy_by_default.xml deleted file mode 100644 index cab8a106f1b8..000000000000 --- a/tests/config/config.d/azure_storage_policy_by_default.xml +++ /dev/null @@ -1,5 +0,0 @@ - - - azure_cache - - diff --git a/tests/integration/test_s3_zero_copy_replication/configs/config.d/s3.xml b/tests/integration/test_s3_zero_copy_replication/configs/config.d/s3.xml index 8df9e8e8c260..3a2bec7f314f 100644 --- a/tests/integration/test_s3_zero_copy_replication/configs/config.d/s3.xml +++ b/tests/integration/test_s3_zero_copy_replication/configs/config.d/s3.xml @@ -7,21 +7,21 @@ http://minio1:9001/root/data/ minio minio123 - true + false s3 http://minio1:9001/root/data/ minio minio123 - true + false s3 http://minio1:9001/root/data2/ minio minio123 - true + false diff --git a/tests/integration/test_s3_zero_copy_replication/test.py b/tests/integration/test_s3_zero_copy_replication/test.py index 0ca81a278021..913da707c18c 100644 --- a/tests/integration/test_s3_zero_copy_replication/test.py +++ b/tests/integration/test_s3_zero_copy_replication/test.py @@ -1,9 +1,11 @@ import datetime import logging +import threading import time import pytest from helpers.cluster import ClickHouseCluster +from helpers.network import PartitionManager logging.getLogger().setLevel(logging.INFO) logging.getLogger().addHandler(logging.StreamHandler()) @@ -76,15 +78,19 @@ def wait_for_large_objects_count(cluster, expected, size=100, timeout=30): assert get_large_objects_count(cluster, size=size) == expected -def wait_for_active_parts(node, num_expected_parts, table_name, timeout=30): +def wait_for_active_parts( + node, num_expected_parts, table_name, timeout=30, disk_name=None +): deadline = time.monotonic() + timeout num_parts = 0 while time.monotonic() < deadline: - num_parts_str = node.query( - "select count() from system.parts where table = '{}' and active".format( - table_name - ) + query = ( + f"select count() from system.parts where table = '{table_name}' and active" ) + if disk_name: + query += f" and disk_name='{disk_name}'" + + num_parts_str = node.query(query) num_parts = int(num_parts_str.strip()) if num_parts == num_expected_parts: return @@ -94,6 +100,22 @@ def wait_for_active_parts(node, num_expected_parts, table_name, timeout=30): assert num_parts == num_expected_parts +@pytest.fixture(scope="function") +def test_name(request): + return request.node.name + + +@pytest.fixture(scope="function") +def test_table(test_name): + normalized = ( + test_name.replace("[", "_") + .replace("]", "_") + .replace(" ", "_") + .replace("-", "_") + ) + return "table_" + normalized + + # Result of `get_large_objects_count` can be changed in other tests, so run this case at the beginning @pytest.mark.order(0) @pytest.mark.parametrize("policy", ["s3"]) @@ -667,3 +689,111 @@ def test_s3_zero_copy_keeps_data_after_mutation(started_cluster): time.sleep(10) check_objects_not_exisis(cluster, objectsY) + + +@pytest.mark.parametrize( + "failpoint", ["zero_copy_lock_zk_fail_before_op", "zero_copy_lock_zk_fail_after_op"] +) +def test_move_shared_lock_fail_once(started_cluster, test_table, failpoint): + node1 = cluster.instances["node1"] + node2 = cluster.instances["node2"] + + node1.query( + f""" + CREATE TABLE {test_table} ON CLUSTER test_cluster (num UInt64, date DateTime) + ENGINE=ReplicatedMergeTree('/clickhouse/tables/{test_table}', '{{replica}}') + ORDER BY date PARTITION BY date + SETTINGS storage_policy='hybrid' + """ + ) + + date = "2024-10-23" + + node2.query(f"SYSTEM STOP FETCHES {test_table}") + node1.query(f"INSERT INTO {test_table} VALUES (1, '{date}')") + + # Try to move and get fail on acquring zero-copy shared lock + node1.query(f"SYSTEM ENABLE FAILPOINT {failpoint}") + node1.query_and_get_error( + f"ALTER TABLE {test_table} MOVE PARTITION '{date}' TO VOLUME 'external'" + ) + + # After fail the part must remain on the source disk + assert ( + node1.query( + f"SELECT disk_name FROM system.parts WHERE table='{test_table}' GROUP BY disk_name" + ) + == "default\n" + ) + + # Try another attempt after zk connection is restored + # It should not failed due to leftovers of previous attempt (temporary cloned files) + node1.query(f"SYSTEM DISABLE FAILPOINT {failpoint}") + node1.query( + f"ALTER TABLE {test_table} MOVE PARTITION '{date}' TO VOLUME 'external'" + ) + + assert ( + node1.query( + f"SELECT disk_name FROM system.parts WHERE table='{test_table}' GROUP BY disk_name" + ) + == "s31\n" + ) + + # Sanity check + node2.query(f"SYSTEM START FETCHES {test_table}") + wait_for_active_parts(node2, 1, test_table, disk_name="s31") + assert node2.query(f"SELECT sum(num) FROM {test_table}") == "1\n" + + node1.query(f"DROP TABLE IF EXISTS {test_table} SYNC") + node2.query(f"DROP TABLE IF EXISTS {test_table} SYNC") + + +def test_move_shared_lock_fail_keeper_unavailable(started_cluster, test_table): + node1 = cluster.instances["node1"] + node2 = cluster.instances["node2"] + + node1.query( + f""" + CREATE TABLE {test_table} ON CLUSTER test_cluster (num UInt64, date DateTime) + ENGINE=ReplicatedMergeTree('/clickhouse/tables/{test_table}', '{{replica}}') + ORDER BY date PARTITION BY date + SETTINGS storage_policy='hybrid' + """ + ) + + date = "2024-10-23" + node2.query(f"SYSTEM STOP FETCHES {test_table}") + + node1.query(f"INSERT INTO {test_table} VALUES (1, '{date}')") + # Pause moving after part cloning, but before swapping + node1.query("SYSTEM ENABLE FAILPOINT stop_moving_part_before_swap_with_active") + + def move(node): + node.query_and_get_error( + f"ALTER TABLE {test_table} MOVE PARTITION '{date}' TO VOLUME 'external'" + ) + + # Start moving + t1 = threading.Thread(target=move, args=[node1]) + t1.start() + + with PartitionManager() as pm: + pm.drop_instance_zk_connections(node1) + # Continue moving and try to swap + node1.query("SYSTEM DISABLE FAILPOINT stop_moving_part_before_swap_with_active") + t1.join() + + # Previous MOVE was failed, try another one after zk connection is restored + # It should not failed due to leftovers of previous attempt (temporary cloned files) + node1.query_with_retry( + f"ALTER TABLE {test_table} MOVE PARTITION '{date}' TO VOLUME 'external'" + ) + + # Sanity check + node2.query(f"SYSTEM START FETCHES {test_table}") + wait_for_active_parts(node2, 1, test_table, disk_name="s31") + assert node2.query(f"SELECT sum(num) FROM {test_table}") == "1\n" + + node1.query(f"DROP TABLE IF EXISTS {test_table} SYNC") + node2.query(f"DROP TABLE IF EXISTS {test_table} SYNC") diff --git a/tests/integration/test_unambiguous_alter_commands/test.py b/tests/integration/test_unambiguous_alter_commands/test.py index 768ab78fbd8d..e7b9fb37c40a 100644 --- a/tests/integration/test_unambiguous_alter_commands/test.py +++ b/tests/integration/test_unambiguous_alter_commands/test.py @@ -43,3 +43,10 @@ def test_alter(): """ result = node.query(INPUT) assert result == EXPECTED_OUTPUT + + +def test_move_partition_to_table_command(): + INPUT = "SELECT formatQuery('ALTER TABLE a MOVE PARTITION tuple() TO TABLE b')" + EXPECTED_OUTPUT = "ALTER TABLE a\\n (MOVE PARTITION tuple() TO TABLE b)\n" + result = node.query(INPUT) + assert result == EXPECTED_OUTPUT diff --git a/tests/queries/0_stateless/03271_decimal_monotonic_day_of_week.reference b/tests/queries/0_stateless/03271_decimal_monotonic_day_of_week.reference new file mode 100644 index 000000000000..b8626c4cff28 --- /dev/null +++ b/tests/queries/0_stateless/03271_decimal_monotonic_day_of_week.reference @@ -0,0 +1 @@ +4 diff --git a/tests/queries/0_stateless/03271_decimal_monotonic_day_of_week.sql b/tests/queries/0_stateless/03271_decimal_monotonic_day_of_week.sql new file mode 100644 index 000000000000..66e085c502d6 --- /dev/null +++ b/tests/queries/0_stateless/03271_decimal_monotonic_day_of_week.sql @@ -0,0 +1,7 @@ +DROP TABLE IF EXISTS decimal_dt; + +CREATE TABLE decimal_dt (timestamp DateTime64(9)) ENGINE=MergeTree() ORDER BY timestamp; +INSERT INTO decimal_dt VALUES (toDate('2024-11-11')),(toDate('2024-11-12')),(toDate('2024-11-13')),(toDate('2024-11-14')),(toDate('2024-11-15')),(toDate('2024-11-16')),(toDate('2024-11-17')); +SELECT count() FROM decimal_dt WHERE toDayOfWeek(timestamp) > 3; + +DROP TABLE IF EXISTS decimal_dt; diff --git a/tests/queries/0_stateless/03272_partition_pruning_monotonic_func_bug.reference b/tests/queries/0_stateless/03272_partition_pruning_monotonic_func_bug.reference new file mode 100644 index 000000000000..cbb68477a293 --- /dev/null +++ b/tests/queries/0_stateless/03272_partition_pruning_monotonic_func_bug.reference @@ -0,0 +1,11 @@ +1 +2 +Expression + ReadFromMergeTree + Indexes: + PrimaryKey + Keys: + dateTrunc(\'hour\', ts) + Condition: and((dateTrunc(\'hour\', ts) in (-Inf, 1731592800]), (dateTrunc(\'hour\', ts) in [1731506400, +Inf))) + Parts: 1/1 + Granules: 1/1 diff --git a/tests/queries/0_stateless/03272_partition_pruning_monotonic_func_bug.sql b/tests/queries/0_stateless/03272_partition_pruning_monotonic_func_bug.sql new file mode 100644 index 000000000000..b69b7b8a7541 --- /dev/null +++ b/tests/queries/0_stateless/03272_partition_pruning_monotonic_func_bug.sql @@ -0,0 +1,19 @@ +SET session_timezone = 'Etc/UTC'; + +DROP TABLE IF EXISTS tt; +CREATE TABLE tt +( + `id` Int64, + `ts` DateTime +) +ENGINE = MergeTree() +ORDER BY dateTrunc('hour', ts) +SETTINGS index_granularity = 8192; + +INSERT INTO tt VALUES (1, '2024-11-14 00:00:00'), (2, '2024-11-14 00:00:00'); + +SELECT id FROM tt PREWHERE ts BETWEEN toDateTime(1731506400) AND toDateTime(1731594420); + +explain indexes=1, description=0 SELECT id FROM tt PREWHERE ts BETWEEN toDateTime(1731506400) AND toDateTime(1731594420); + +DROP TABLE tt; diff --git a/tests/queries/0_stateless/03199_dictionary_table_access.reference b/tests/queries/0_stateless/03273_dictionary_rbac.reference similarity index 50% rename from tests/queries/0_stateless/03199_dictionary_table_access.reference rename to tests/queries/0_stateless/03273_dictionary_rbac.reference index 4a703b3be841..f7ff98213bac 100644 --- a/tests/queries/0_stateless/03199_dictionary_table_access.reference +++ b/tests/queries/0_stateless/03273_dictionary_rbac.reference @@ -1,2 +1,4 @@ -ACCESS_DENIED +Ok. +Ok. +Ok. ACCESS_DENIED diff --git a/tests/queries/0_stateless/03199_dictionary_table_access.sh b/tests/queries/0_stateless/03273_dictionary_rbac.sh similarity index 53% rename from tests/queries/0_stateless/03199_dictionary_table_access.sh rename to tests/queries/0_stateless/03273_dictionary_rbac.sh index 14f017c7fbcb..d74039b4d611 100755 --- a/tests/queries/0_stateless/03199_dictionary_table_access.sh +++ b/tests/queries/0_stateless/03273_dictionary_rbac.sh @@ -8,7 +8,8 @@ username="user_${CLICKHOUSE_TEST_UNIQUE_NAME}" dictname="dict_${CLICKHOUSE_TEST_UNIQUE_NAME}" dicttablename="dict_table_${CLICKHOUSE_TEST_UNIQUE_NAME}" -${CLICKHOUSE_CLIENT} -m --query " +# Create a dictionary and a table that points to the dictionary. +${CLICKHOUSE_CLIENT} -nm --query " CREATE DICTIONARY IF NOT EXISTS ${dictname} ( id UInt64, @@ -18,23 +19,37 @@ ${CLICKHOUSE_CLIENT} -m --query " SOURCE(NULL()) LAYOUT(FLAT()) LIFETIME(MIN 0 MAX 1000); - CREATE USER IF NOT EXISTS ${username} NOT IDENTIFIED; - GRANT SELECT, CREATE TEMPORARY TABLE ON *.* to ${username}; - SELECT * FROM ${dictname}; CREATE TABLE ${dicttablename} (id UInt64, value UInt64) ENGINE = Dictionary(${CLICKHOUSE_DATABASE}.${dictname}); - SELECT * FROM ${dicttablename}; " -$CLICKHOUSE_CLIENT -m --user="${username}" --query " +# Create a user, assign only a few permissions. +${CLICKHOUSE_CLIENT} -nm --query " + CREATE USER IF NOT EXISTS ${username} NOT IDENTIFIED; + GRANT SELECT, CREATE TEMPORARY TABLE ON *.* to ${username}; +" + +# Reading from dictionary via direct SELECT is Ok. +$CLICKHOUSE_CLIENT -nm --user="${username}" --query " SELECT * FROM ${dictname}; -" 2>&1 | grep -o ACCESS_DENIED | uniq +" >/dev/null 2>&1 && echo "Ok." -$CLICKHOUSE_CLIENT -m --user="${username}" --query " +# Reading from dictionary via dictionary storage is Ok. +$CLICKHOUSE_CLIENT -nm --user="${username}" --query " SELECT * FROM ${dicttablename}; +" >/dev/null 2>&1 && echo "Ok." + +# Reading from dictionary via dictionary table function is Ok. +$CLICKHOUSE_CLIENT -nm --user="${username}" --query " + SELECT * FROM dictionary(${dictname}); +" >/dev/null 2>&1 && echo "Ok." + +# Function dictGet requires a permission dictGet to use. +$CLICKHOUSE_CLIENT -nm --user="${username}" --query " + SELECT dictGet(${dictname}, 'value', 1); " 2>&1 | grep -o ACCESS_DENIED | uniq -${CLICKHOUSE_CLIENT} -m --query " +${CLICKHOUSE_CLIENT} -nm --query " DROP TABLE IF EXISTS ${dicttablename} SYNC; DROP DICTIONARY IF EXISTS ${dictname}; DROP USER IF EXISTS ${username}; diff --git a/tests/queries/0_stateless/03273_select_from_explain_ast_non_select.reference b/tests/queries/0_stateless/03273_select_from_explain_ast_non_select.reference new file mode 100644 index 000000000000..83830f7b4481 --- /dev/null +++ b/tests/queries/0_stateless/03273_select_from_explain_ast_non_select.reference @@ -0,0 +1,11 @@ +SelectWithUnionQuery (children 1) + ExpressionList (children 1) + SelectQuery (children 2) + ExpressionList (children 1) + Asterisk + TablesInSelectQuery (children 1) + TablesInSelectQueryElement (children 1) + TableExpression (children 1) + Function numbers (children 1) + ExpressionList (children 1) + Literal UInt64_10 diff --git a/tests/queries/0_stateless/03273_select_from_explain_ast_non_select.sql b/tests/queries/0_stateless/03273_select_from_explain_ast_non_select.sql new file mode 100644 index 000000000000..7298186414b1 --- /dev/null +++ b/tests/queries/0_stateless/03273_select_from_explain_ast_non_select.sql @@ -0,0 +1,5 @@ +SELECT * FROM ( EXPLAIN AST SELECT * FROM numbers(10) ); +SELECT * FROM ( EXPLAIN AST CREATE TABLE test ENGINE=Memory ); -- {clientError BAD_ARGUMENTS} +SELECT * FROM ( EXPLAIN AST CREATE MATERIALIZED VIEW mv (data String) AS SELECT data FROM table ); -- {clientError BAD_ARGUMENTS} +SELECT * FROM ( EXPLAIN AST INSERT INTO TABLE test VALUES); -- {clientError BAD_ARGUMENTS} +SELECT * FROM ( EXPLAIN AST ALTER TABLE test MODIFY COLUMN x UInt32 ); -- {clientError BAD_ARGUMENTS}