Skip to content
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

Distributed DDL improvements #21535

Merged
merged 15 commits into from
Mar 29, 2021
Merged

Distributed DDL improvements #21535

merged 15 commits into from
Mar 29, 2021

Conversation

tavplubix
Copy link
Member

@tavplubix tavplubix commented Mar 9, 2021

I hereby agree to the terms of the CLA available at: https://yandex.ru/legal/cla/?lang=en

Changelog category (leave one):

  • Improvement

Changelog entry (a user-readable short description of the changes that goes to CHANGELOG.md):

  • Propagate query and session settings for distributed DDL queries. Set distributed_ddl_entry_format_version to 2 to enable this.
  • Added distributed_ddl_output_mode setting. Supported modes: none, throw (default), null_status_on_timeout and never_throw.
  • Miscellaneous fixes and improvements for Replicated database engine.

Detailed description / Documentation draft:
When distributed_ddl_output_mode is set to throw distributed DDL query returns result set with query execution status for all hosts where query is finished. If query has failed on some hosts, then it will rethrow the first exception. If query is not finished yet on some hosts and distributed_ddl_task_timeout exceeded, then it throws TIMEOUT_EXCEEDED exception.

none is similar to throw, but distributed DDL query returns no result set.

null_status_on_timeout means return NULL as execution status in some rows of result set instead of throwing TIMEOUT_EXCEEDED if query is not finished on the corresponding hosts.

never_throw means do not throw TIMEOUT_EXCEEDED and do not rethrow exceptions if query has failed on some hosts.

@robot-clickhouse robot-clickhouse added the pr-improvement Pull request with some product improvements label Mar 9, 2021
@tavplubix tavplubix marked this pull request as ready for review March 9, 2021 17:54
@nikitamikhaylov nikitamikhaylov self-assigned this Mar 29, 2021
}

ClusterPtr new_cluster = getClusterImpl();
std::lock_guard lock{mutex};
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe move this line above previous? Because many threads will try construct a cluster... And it seems that you can lock mutex only once at the beginning of the method without additional scopes.

@@ -120,7 +141,7 @@ ClusterPtr DatabaseReplicated::getCluster() const
hosts = zookeeper->getChildren(zookeeper_path + "/replicas", &stat);
if (hosts.empty())
throw Exception(ErrorCodes::LOGICAL_ERROR, "No hosts found");
Int32 cver = stat.cversion;
Int32 cversion = stat.cversion;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is there any useful information inside /replicas node itself (without children)? Maybe better to check version in addition to cversion?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As for now /replicas node does not store any informatin

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
pr-improvement Pull request with some product improvements
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants