Skip to content

Fix no shared id during drop for the fourth time#45363

Merged
tavplubix merged 10 commits intomasterfrom
fix_hang_during_drop_in_zero_copy_replication
Jan 27, 2023
Merged

Fix no shared id during drop for the fourth time#45363
tavplubix merged 10 commits intomasterfrom
fix_hang_during_drop_in_zero_copy_replication

Conversation

@alesapin
Copy link
Copy Markdown
Member

Changelog category (leave one):

  • Not for changelog (changelog entry is not required)

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

Fixes #45224.

@robot-ch-test-poll3 robot-ch-test-poll3 added the pr-not-for-changelog This PR should not be mentioned in the changelog label Jan 17, 2023
@tavplubix tavplubix self-assigned this Jan 17, 2023
Comment on lines +7940 to +7942
else if (code == Coordination::Error::ZNONODE) /// table completely dropped, we can choose any id we want
{
LOG_DEBUG(log, "Table was completely drop, we can use anything as ID (will use {})", id);
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Why do we need an id if the table is completely dropped? What are we going to unlock in unlockSharedDataByID in this case?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

We will just check that this ID doesn't exist in ZK and assume parts unlocked, but I'm not 100% sure...

Comment on lines +7968 to +7970
else if (code == Coordination::Error::ZNONODE) /// table completely dropped, we can choose any id we want
{
LOG_DEBUG(log, "Table was completely drop, we can use anything as ID (will use {})", id);
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Let's return Nil in this case

@tavplubix
Copy link
Copy Markdown
Member

Integration tests (release) [4/4] - looks like #44862
Stress test (debug) - #45691
Stress test (ubsan) - reportLogStats - fixed in #45527

@tavplubix tavplubix merged commit f375b12 into master Jan 27, 2023
@tavplubix tavplubix deleted the fix_hang_during_drop_in_zero_copy_replication branch January 27, 2023 12:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

pr-not-for-changelog This PR should not be mentioned in the changelog

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Stress: DROP TABLE may hang with zero-copy replication brecause of table_shared_id

3 participants