Try fix 'some fetches may stuck' #30346
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
robot-clickhouse
added
the
pr-improvement
alesapin
approved these changes
Oct 22, 2021
| LogEntryPtr parsed_entry = {}; | ||
| }; | ||
|
|
||
| std::vector<QueueEntryInfo> source_queue; |
There was a problem hiding this comment.
Need comment with general idea.
| String path_created = dynamic_cast<const Coordination::CreateResponse &>(*results.back()).path_created; | ||
| log_entry->znode_name = path_created.substr(path_created.find_last_of('/') + 1); | ||
| queue.insert(zookeeper, log_entry); | ||
| zookeeper->multi(ops); |
There was a problem hiding this comment.
We don't need to hack queue here, because new entry will pulled in pullLogsToQueue?
There was a problem hiding this comment.
Actually this change was incorrect, because we cannot pull entries from queue to queue
| /// but before we copied its active parts set. In this case we will GET_PART entry in our queue | ||
| /// and later will pull the original GET_PART from replication log. | ||
| /// It should not cause any issues, but it does not allow to get rid of duplicated entries and add an assertion. | ||
| if (created_get_parts.count(part_name)) |
There was a problem hiding this comment.
Maybe pass created_get_parts as argument. Currently it's hard to understand that should_ignore_log_entry depends on it.
| @@ -324,7 +326,8 @@ class ReplicatedMergeTreeQueue | |||
| /** Remove the action from the queue with the parts covered by part_name (from ZK and from the RAM). | |||
| * And also wait for the completion of their execution, if they are now being executed. | |||
| */ | |||
| void removePartProducingOpsInRange(zkutil::ZooKeeperPtr zookeeper, const MergeTreePartInfo & part_info, const ReplicatedMergeTreeLogEntryData & current); | |||
| void removePartProducingOpsInRange(zkutil::ZooKeeperPtr zookeeper, const MergeTreePartInfo & part_info, | |||
| const std::optional<ReplicatedMergeTreeLogEntryData> & current); | |||
There was a problem hiding this comment.
What is current? Maybe add a comment?
|
Functional stateless tests (debug) - race between |
This was referenced Nov 1, 2021
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Changelog category (leave one):
Changelog entry (a user-readable short description of the changes that goes to CHANGELOG.md):
Minor improvements in replica cloning and enqueuing fetch for broken parts, that should avoid extremely rare hanging of
GET_PARTentries in replication queue.