Skip to content

[fix][meta] Complete handleMetadataEvent future exceptionally when the initial get fails#26199

Open
SongOf wants to merge 1 commit into
apache:masterfrom
SongOf:fix/metadata-handle-metadata-event-hanging-future
Open

[fix][meta] Complete handleMetadataEvent future exceptionally when the initial get fails#26199
SongOf wants to merge 1 commit into
apache:masterfrom
SongOf:fix/metadata-handle-metadata-event-hanging-future

Conversation

@SongOf

@SongOf SongOf commented Jul 16, 2026

Copy link
Copy Markdown
Contributor

Motivation

AbstractMetadataStore#handleMetadataEvent chains the initial
get(event.getPath()) with thenApply, but the outer chain has no
handler for exceptional completion. When that get fails (e.g. the
backing store is unavailable), the thenApply callback is skipped and
the result future returned to the caller never completes.

The caller is the MetadataEventSynchronizer listener path
(PulsarMetadataEventSynchronizer's consumer message listener): with a
hanging future, neither the acknowledgeAsync stage nor the
exceptionally stage ever runs, so the event message is not
acknowledged, the failure cause is never logged, and every ack-timeout
redelivery registers another never-completing future. The inner
updateResult chain already handles exceptional completion; only the
outer get chain was missing it.

Modifications

  • AbstractMetadataStore#handleMetadataEvent: switch the outer chain
    from thenApply (whose mapped value was never used) to thenAccept,
    and append an exceptionally stage that unwraps the
    CompletionException via FutureUtil.unwrapCompletionException,
    logs a warning, and completes the result future exceptionally with
    the root cause.
  • The new exceptionally stage also covers exceptions thrown
    synchronously inside the callback body (e.g. from
    shouldIgnoreEvent), which previously left the future hanging the
    same way.
  • The existing behavior on the success path is unchanged, including the
    inner updateResult handling (BadVersionException is still treated
    as success).

Verifying this change

  • Make sure that the change passes the CI checks.

This change added tests and can be verified as follows:

- Added `MetadataEventSynchronizerTest#testHandleMetadataEventCompletesWhenGetFails`,
  which injects a failing `storeGet` into a `LocalMemoryMetadataStore`
  subclass and asserts that the future returned by
  `handleMetadataEvent` completes exceptionally (with a
  `MetadataStoreException` cause) within a timeout. On the unpatched
  code the test fails with a `TimeoutException`, reproducing the hang;
  with the fix it passes.

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

If the box was checked, please highlight the changes

  • Dependencies (add or upgrade a dependency)
  • The public API
  • 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

This is an internal behavior change: the future returned by
handleMetadataEvent now completes exceptionally instead of hanging
when the initial read fails. No public API, schema, configuration,
wire protocol, REST endpoint, CLI option, or metric is affected.

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.

1 participant