-
Notifications
You must be signed in to change notification settings - Fork 18
Allow empty object storage cluster #2028
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: antalya-26.3
Are you sure you want to change the base?
Changes from 8 commits
93e7bdc
0297ff2
8bac602
bc7aacf
1ad4ce9
195f687
92a5142
13eaae5
26cb88a
9d91e1f
988e463
20fa735
86ff083
9e1b15f
4ba13df
4b41b1d
afbd078
e0c3a5d
e8b4414
7c56562
8e1ba58
aaab89d
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -57,6 +57,7 @@ namespace Setting | |
| extern const SettingsBool object_storage_remote_initiator; | ||
| extern const SettingsString object_storage_remote_initiator_cluster; | ||
| extern const SettingsObjectStorageClusterJoinMode object_storage_cluster_join_mode; | ||
| extern const SettingsBool object_storage_cluster_fallback_if_empty; | ||
| } | ||
|
|
||
| namespace ErrorCodes | ||
|
|
@@ -344,6 +345,62 @@ void IStorageCluster::updateQueryWithJoinToSendIfNeeded( | |
| } | ||
| } | ||
|
|
||
| IStorageCluster::ResolvedClusterRead IStorageCluster::resolveClusterRead(ContextPtr context) const | ||
| { | ||
| ResolvedClusterRead result; | ||
|
|
||
| if (!isClusterSupported()) | ||
| { | ||
| result.fallback_to_pure = true; | ||
| return result; | ||
| } | ||
|
|
||
| auto cluster_name_from_settings = getClusterName(context); | ||
| const auto & settings = context->getSettingsRef(); | ||
|
|
||
| /// When both remote-initiator settings are set, object_storage_cluster may be defined only on the remote node. | ||
| /// In this case object_storage_cluster must not be resolved locally. | ||
| const bool defer_object_storage_cluster_resolution | ||
| = settings[Setting::object_storage_remote_initiator] | ||
| && !settings[Setting::object_storage_remote_initiator_cluster].value.empty(); | ||
|
Comment on lines
+404
to
+406
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
When Useful? React with 👍 / 👎. |
||
|
|
||
| if (defer_object_storage_cluster_resolution) | ||
| result.fallback_to_pure = usePureFunctionForRemoteInitiator(context); | ||
| else | ||
| result.fallback_to_pure = cluster_name_from_settings.empty(); | ||
|
|
||
| if (!defer_object_storage_cluster_resolution | ||
| && useObjectStorageClusterFallbackIfEmpty(context) | ||
| && !result.fallback_to_pure | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think there is a naming conflict / confusion here. As far as I understand, there are two distinct concepts you are calling "fallback". Fallback to pure means read as a plain s3 function afaik, while fallback to local means perform the operation locally. I guess too many "fallback", maybe one of them should be named fallback |
||
| && settings[Setting::object_storage_cluster_fallback_if_empty]) | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Because this check is in the Useful? React with 👍 / 👎. |
||
| { | ||
| result.object_storage_cluster = getClusterImpl( | ||
| context, | ||
| cluster_name_from_settings, | ||
| isObjectStorage() ? settings[Setting::object_storage_max_nodes] : 0, | ||
| /*allow_null*/ true); | ||
| if (!result.object_storage_cluster) | ||
| result.fallback_to_pure = true; | ||
| } | ||
|
|
||
| if (settings[Setting::object_storage_remote_initiator]) | ||
| { | ||
| auto remote_initiator_cluster_name = settings[Setting::object_storage_remote_initiator_cluster].value; | ||
| if (!remote_initiator_cluster_name.empty()) | ||
| { | ||
|
Comment on lines
+430
to
+432
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
When Useful? React with 👍 / 👎. |
||
| const bool allow_null = settings[Setting::object_storage_cluster_fallback_if_empty] | ||
| && (result.fallback_to_pure || usePureFunctionForRemoteInitiator(context)); | ||
| result.remote_initiator_cluster = getClusterImpl( | ||
| context, | ||
| remote_initiator_cluster_name, | ||
| /*max_hosts*/ 0, | ||
| allow_null); | ||
| } | ||
| } | ||
|
|
||
| return result; | ||
| } | ||
|
|
||
| /// The code executes on initiator | ||
| void IStorageCluster::read( | ||
| QueryPlan & query_plan, | ||
|
|
@@ -365,23 +422,36 @@ void IStorageCluster::read( | |
| const auto & settings = context->getSettingsRef(); | ||
| ASTPtr query_to_send = query_info.query; | ||
|
|
||
| if (cluster_name_from_settings.empty()) | ||
| auto resolved = resolveClusterRead(context); | ||
| ClusterPtr cluster = resolved.object_storage_cluster; | ||
|
|
||
| const bool send_pure_function_to_remote_initiator | ||
| = settings[Setting::object_storage_remote_initiator] && usePureFunctionForRemoteInitiator(context); | ||
|
|
||
| if (resolved.fallback_to_pure || send_pure_function_to_remote_initiator) | ||
| { | ||
| if (settings[Setting::object_storage_remote_initiator]) | ||
| { | ||
| auto remote_initiator_cluster_name = settings[Setting::object_storage_remote_initiator_cluster].value; | ||
| if (remote_initiator_cluster_name.empty()) | ||
| if (!resolved.remote_initiator_cluster) | ||
| { | ||
| if (settings[Setting::object_storage_cluster_fallback_if_empty]) | ||
| { | ||
| readFallBackToPure(query_plan, column_names, storage_snapshot, query_info, context, processed_stage, max_block_size, num_streams); | ||
| return; | ||
| } | ||
| throw Exception(ErrorCodes::BAD_ARGUMENTS, | ||
| "Setting 'object_storage_remote_initiator' can be used only with 'object_storage_remote_initiator_cluster', 'object_storage_cluster', or cluster name in arguments"); | ||
| } | ||
|
|
||
| auto remote_initiator_cluster_name = settings[Setting::object_storage_remote_initiator_cluster].value; | ||
|
|
||
| /// rewrite query to execute `remote('remote_host', s3(...))` | ||
| /// remote_host can execute query itself or make on-cluster query depends on own `object_storage_cluster` setting | ||
| updateConfigurationIfNeeded(context); | ||
| updateQueryWithJoinToSendIfNeeded(query_to_send, query_info, context); | ||
| updateQueryToSendIfNeeded(query_to_send, storage_snapshot, context, /*make_cluster_function*/ false); | ||
|
|
||
| auto remote_initiator_cluster = getClusterImpl(context, remote_initiator_cluster_name); | ||
| auto storage_and_context = convertToRemote(remote_initiator_cluster, context, remote_initiator_cluster_name, query_to_send); | ||
| auto storage_and_context = convertToRemote(resolved.remote_initiator_cluster, context, remote_initiator_cluster_name, query_to_send); | ||
| auto src_distributed = std::dynamic_pointer_cast<StorageDistributed>(storage_and_context.storage); | ||
| auto modified_query_info = query_info; | ||
| modified_query_info.cluster = src_distributed->getCluster(); | ||
|
|
@@ -440,7 +510,8 @@ void IStorageCluster::read( | |
| return; | ||
| } | ||
|
|
||
| auto cluster = getClusterImpl(context, cluster_name_from_settings, isObjectStorage() ? settings[Setting::object_storage_max_nodes] : 0); | ||
| if (!cluster) | ||
| cluster = getClusterImpl(context, cluster_name_from_settings, isObjectStorage() ? settings[Setting::object_storage_max_nodes] : 0); | ||
|
|
||
| RestoreQualifiedNamesVisitor::Data data; | ||
| data.distributed_table = DatabaseAndTableWithAlias(*getTableExpression(query_to_send->as<ASTSelectQuery &>(), 0)); | ||
|
|
@@ -564,6 +635,8 @@ SinkToStoragePtr IStorageCluster::write( | |
| { | ||
| auto cluster_name_from_settings = getClusterName(context); | ||
|
|
||
| // Intentionally do not apply object_storage_cluster_fallback_if_empty here. | ||
| // Cluster write is not supported; applying fallback would make INSERT depend on cluster availability. | ||
| if (cluster_name_from_settings.empty()) | ||
| return writeFallBackToPure(query, metadata_snapshot, context, async_insert); | ||
|
|
||
|
|
@@ -730,9 +803,18 @@ ContextPtr ReadFromCluster::updateSettings(const Settings & settings) | |
| return new_context; | ||
| } | ||
|
|
||
| ClusterPtr IStorageCluster::getClusterImpl(ContextPtr context, const String & cluster_name_, size_t max_hosts) | ||
| ClusterPtr IStorageCluster::getClusterImpl(ContextPtr context, const String & cluster_name_, size_t max_hosts, bool allow_null) | ||
| { | ||
| return context->getCluster(cluster_name_)->getClusterWithReplicasAsShards(context->getSettingsRef(), /* max_replicas_from_shard */ 0, max_hosts); | ||
| ClusterPtr cluster = nullptr; | ||
| if (allow_null) | ||
| { | ||
| cluster = context->tryGetCluster(cluster_name_); | ||
| if (!cluster || !cluster->getAllNodeCount()) | ||
| return nullptr; | ||
| } | ||
| else | ||
| cluster = context->getCluster(cluster_name_); | ||
| return cluster->getClusterWithReplicasAsShards(context->getSettingsRef(), /* max_replicas_from_shard */ 0, max_hosts); | ||
| } | ||
|
|
||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -42,6 +42,7 @@ namespace Setting | |
| extern const SettingsInt64 delta_lake_snapshot_end_version; | ||
| extern const SettingsUInt64 lock_object_storage_task_distribution_ms; | ||
| extern const SettingsBool allow_experimental_iceberg_read_optimization; | ||
| extern const SettingsBool object_storage_cluster_fallback_if_empty; | ||
| } | ||
|
|
||
| namespace ErrorCodes | ||
|
|
@@ -706,17 +707,31 @@ SinkToStoragePtr StorageObjectStorageCluster::writeFallBackToPure( | |
| String StorageObjectStorageCluster::getClusterName(ContextPtr context) const | ||
| { | ||
| /// StorageObjectStorageCluster is always created for cluster or non-cluster variants. | ||
| /// User can specify cluster name in table definition or in setting `object_storage_cluster` | ||
| /// only for several queries. When it specified in both places, priority is given to the query setting. | ||
| /// When it is empty, non-cluster realization is used. | ||
| /// User can specify cluster name in table definition, in *Cluster table function argument, | ||
| /// or in setting `object_storage_cluster` for s3()/iceberg() alternative syntax. | ||
| /// Explicit *Cluster argument has priority over query setting; table engine and alternative-syntax use the setting path. | ||
|
|
||
| if (!isClusterSupported()) | ||
| return ""; | ||
|
|
||
| if (cluster_name_from_function_argument) | ||
| return getOriginalClusterName(); | ||
|
|
||
| auto cluster_name_from_settings = context->getSettingsRef()[Setting::object_storage_cluster].value; | ||
| if (cluster_name_from_settings.empty()) | ||
| cluster_name_from_settings = getOriginalClusterName(); | ||
| return cluster_name_from_settings; | ||
| if (!cluster_name_from_settings.empty()) | ||
| return cluster_name_from_settings; | ||
|
|
||
| return getOriginalClusterName(); | ||
| } | ||
|
|
||
| bool StorageObjectStorageCluster::useObjectStorageClusterFallbackIfEmpty(ContextPtr context) const | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It seems like I don't understand this PR enough. From the description, I got the idea that if the remote cluster isn't available (i.e, no nodes alive), the setting would allow to execute it on this machine. But the method
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Agree, I'll change it on more clean code. |
||
| { | ||
| if (cluster_name_from_function_argument) | ||
| return false; | ||
|
|
||
| return cluster_name_in_settings | ||
| || !getOriginalClusterName().empty() | ||
| || !context->getSettingsRef()[Setting::object_storage_cluster].value.empty(); | ||
| } | ||
|
|
||
| QueryProcessingStage::Enum StorageObjectStorageCluster::getQueryProcessingStage( | ||
|
|
@@ -725,15 +740,29 @@ QueryProcessingStage::Enum StorageObjectStorageCluster::getQueryProcessingStage( | |
| if (!isClusterSupported()) | ||
| return QueryProcessingStage::Enum::FetchColumns; | ||
|
|
||
| /// Full query if fall back to pure storage. | ||
| if (getClusterName(context).empty() // Not cluster request | ||
| && context->getSettingsRef()[Setting::object_storage_remote_initiator_cluster].value.empty()) // Not request with remote initiator | ||
| auto resolved = resolveClusterRead(context); | ||
| const auto & settings = context->getSettingsRef(); | ||
|
|
||
| const bool send_pure_function_to_remote_initiator | ||
| = settings[Setting::object_storage_remote_initiator] && usePureFunctionForRemoteInitiator(context); | ||
|
|
||
| if (resolved.fallback_to_pure || send_pure_function_to_remote_initiator) | ||
| { | ||
| if (context->getSettingsRef()[Setting::object_storage_remote_initiator]) | ||
| throw Exception(ErrorCodes::BAD_ARGUMENTS, | ||
| "Setting 'object_storage_remote_initiator' can be used only with 'object_storage_remote_initiator_cluster', 'object_storage_cluster', or cluster name in arguments"); | ||
| if (settings[Setting::object_storage_remote_initiator]) | ||
| { | ||
| if (!resolved.remote_initiator_cluster) | ||
| { | ||
| if (!settings[Setting::object_storage_cluster_fallback_if_empty]) | ||
| throw Exception(ErrorCodes::BAD_ARGUMENTS, | ||
| "Setting 'object_storage_remote_initiator' can be used only with 'object_storage_remote_initiator_cluster', 'object_storage_cluster', or cluster name in arguments"); | ||
|
|
||
| return QueryProcessingStage::Enum::FetchColumns; | ||
| return QueryProcessingStage::Enum::FetchColumns; | ||
| } | ||
| } | ||
| else | ||
| { | ||
| return QueryProcessingStage::Enum::FetchColumns; | ||
| } | ||
| } | ||
|
|
||
| /// Distributed storage. | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -154,7 +154,10 @@ StoragePtr TableFunctionObjectStorageClusterFallback<Definition, Base>::executeI | |
| { | ||
| auto result = BaseCluster::executeImpl(ast_function, context, table_name, cached_columns, is_insert_query); | ||
| if (auto storage = typeid_cast<std::shared_ptr<StorageObjectStorageCluster>>(result)) | ||
| { | ||
| storage->setClusterNameInSettings(true); | ||
| storage->setClusterNameFromFunctionArgument(false); | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Do you need
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Good catch
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. There are three cases
May be better to make enum with three values instead of two boolean flags.
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Looks like old flag |
||
| } | ||
| return result; | ||
| } | ||
| else | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe rename it to
object_storage_cluster_fallback_to_local_if_empty