Skip to content

[fix][metadata] Fix orphaned UR parent nodes not cleaned up with Oxia metadata backend - #26158

Merged
merlimat merged 2 commits into
apache:masterfrom
eugene-polozhenkov:fix/oxia-underreplication-orphan-cleanup
Jul 7, 2026
Merged

[fix][metadata] Fix orphaned UR parent nodes not cleaned up with Oxia metadata backend#26158
merlimat merged 2 commits into
apache:masterfrom
eugene-polozhenkov:fix/oxia-underreplication-orphan-cleanup

Conversation

@eugene-polozhenkov

Copy link
Copy Markdown
Contributor

Fixes #26157

Motivation

When using Oxia as the metadata backend, markLedgerReplicated() in PulsarLedgerUnderreplicationManager leaves orphaned intermediate nodes in the under-replication (UR) tree after every ledger is successfully replicated.

OxiaMetadataStore.doStorePut() calls createParents() to explicitly create up to 4 intermediate path entries for every UR leaf (Oxia is a flat key-value store; parent nodes must be created explicitly). However, the hierarchy cleanup in markLedgerReplicated() was gated on an instanceof check that excluded OxiaMetadataStore, so those parent nodes were never deleted.

Additionally, the inner catch block only handled ZK's KeeperException.NotEmptyException. Oxia threw a plain MetadataStoreException for the same "node has children" condition, causing a spurious log.warn() on every successfully-replicated ledger.

Over time, orphaned nodes accumulate proportionally to replication activity and cause getLedgerToRereplicateFromHierarchy() to traverse empty subtrees on every replication poll, driving sustained elevated metadata store list ops.

Modifications

  • Add MetadataStoreException.NotEmptyException to the metadata API, consistent with the existing NotFoundException / BadVersionException pattern
  • Map ZK's NOTEMPTY error code to NotEmptyException in ZKMetadataStore.getException() (was falling through to default)
  • Throw NotEmptyException from OxiaMetadataStore.storeDelete() instead of a plain MetadataStoreException, so both backends surface the same exception type
  • Add OxiaMetadataStore to the instanceof guard in markLedgerReplicated()
  • Replace the ZK-specific KeeperException.NotEmptyException check with a clean instanceof MetadataStoreException.NotEmptyException check

Verifying this change

This change added tests and can be verified as follows:

  • Extended testMarkReplicatedDeletesEmptyParentNodes to run against all distributed metadata backends (dataProvider = "distributedImpl": ZK + Oxia) instead of ZK-only (dataProvider = "zkImpls"). The test asserts that all 4 parent hierarchy nodes are deleted after markLedgerReplicated() completes.

Does this pull request potentially affect one of the following parts:

  • Dependencies (add or upgrade a dependency)
  • The public API — added MetadataStoreException.NotEmptyException (new subclass, backward-compatible)
  • The schema
  • The default values of configurations
  • The threading model
  • The binary protocol
  • The REST endpoints
  • The admin CLI options
  • The metrics
  • Anything that affects deployment

@void-ptr974

Copy link
Copy Markdown
Contributor

Do we need to change the ZK NOTEMPTY mapping in this PR? The Oxia issue should be fixable by having Oxia throw MetadataStoreException.NotEmptyException and by making markLedgerReplicated() ignore both the new Oxia exception and the existing ZK shape.

Changing ZK here also changes the exception shape for existing callers. For example, NamespacesBase still checks for KeeperException.NotEmptyException as the cause to retry namespace deletion.

Could we keep the ZK mapping unchanged in this PR, and handle both exception forms in markLedgerReplicated()? If we want to normalize ZK NOTEMPTY too, it may be safer to do that separately with updates to unwrap() and existing catch sites.

@eugene-polozhenkov

Copy link
Copy Markdown
Contributor Author

@void-ptr974 Good catch, thank you. Agreed on all points.
Reverted the ZKMetadataStore NOTEMPTY mapping — that change belongs in a separate PR with updates to unwrap() and existing catch sites (e.g. NamespacesBase).
markLedgerReplicated() now handles both shapes explicitly:

  • Oxia: NotEmptyException thrown directly
  • ZK: MetadataStoreException wrapping KeeperException.NotEmptyException (existing behaviour, unchanged)

@merlimat
merlimat merged commit 8d5cac6 into apache:master Jul 7, 2026
43 checks passed
merlimat pushed a commit that referenced this pull request Jul 7, 2026
… metadata backend (#26158)

Co-authored-by: ievgenpolozhenkov <ievgenpolozhenkov@geotab.com>
(cherry picked from commit 8d5cac6)
Assisted-by: Claude Code (Opus 4.8)
merlimat pushed a commit that referenced this pull request Jul 7, 2026
… metadata backend (#26158)

Co-authored-by: ievgenpolozhenkov <ievgenpolozhenkov@geotab.com>
(cherry picked from commit 8d5cac6)
Assisted-by: Claude Code (Opus 4.8)
lhotari added a commit that referenced this pull request Jul 9, 2026
lhotari added a commit that referenced this pull request Jul 9, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Bug] PulsarLedgerUnderreplicationManager does not clean up orphaned parent nodes when using Oxia metadata backend

4 participants