Skip to content

Fix race condition between table creation and namespace deletion#6476

Open
DomGarguilo wants to merge 1 commit into
apache:mainfrom
DomGarguilo:fixNSdeleteRace
Open

Fix race condition between table creation and namespace deletion#6476
DomGarguilo wants to merge 1 commit into
apache:mainfrom
DomGarguilo:fixNSdeleteRace

Conversation

@DomGarguilo

Copy link
Copy Markdown
Member

Fixes #5389

  • fixes a race condition where NamespaceOperationsImpl.delete() checked if a namespace was empty and then submitted the delete FATE op. If another client tried to create a table in that namespace in between the namespace emptiness check and when the DeleteNamespace was actually executed, the table could still be created and also the namespace deleted.
  • moves the check into the DeleteNamespace.call() itself which happens under a write lock and the race can't happen anymore.
  • added a new NAMESPACE_NOTEMPTY thrift enum so the manager now throws AcceptableThriftTableOperationException with that type which the client catches and converts to NamespaceNotEmptyException
  • added a test to mock the DeleteNamespace call on a non-empty namespace to verify the new exception is thrown

@DomGarguilo DomGarguilo added this to the 4.0.0 milestone Jul 15, 2026
@DomGarguilo DomGarguilo self-assigned this Jul 15, 2026

@phrocker phrocker left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Commenting since I am not a ZK expert and would lean in @DomGarguilo to validate my assertion.

@Override
public Repo<FateEnv> call(FateId fateId, FateEnv environment) {
public Repo<FateEnv> call(FateId fateId, FateEnv environment) throws Exception {
if (!environment.getContext().getTableMapping(namespaceId).getIdToNameMap().isEmpty()) {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I could be wrong, but zk will only guarantee the watch event is queued before a subsequent op response, not that the cache has applied it. The window is small I think. Should we do the emptiness check inside a mutateExisting on the mapping node the way we do in TableMapping.put/remove ? that gets you cas rather than read then act?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Race condition between table creation and namespace deletion

2 participants