lhotari opened a new pull request, #26132:
URL: https://github.com/apache/pulsar/pull/26132
### Motivation
`AdminApi2Test.testGetInternalStatsWithProperties` is flaky. It fails with:
```
java.util.concurrent.ExecutionException:
org.apache.pulsar.broker.service.BrokerServiceException:
Topic policies cache initialization for namespace <ns> was aborted because
the cached state was cleared
```
`SystemTopicBasedTopicPoliciesService.readMorePoliciesAsync` tears down a
namespace's cached policy
state **by namespace key** (`cleanPoliciesCacheInitMap`) when its
`__change_events` reader observes an
`AlreadyClosedException`. That callback runs on the pulsar-client executor,
**not** the per-namespace
ordered executor, so it races a reload:
1. A namespace unload closes reader `R1` asynchronously (via
`cleanPoliciesCacheInitMap` →
`Reader#closeAsync`) and drops its init future.
2. A reload of a topic in the same namespace (e.g. `getTopic` right after
`unload`) installs a fresh
reader `R2` and a fresh, still-pending init future `F2`, and the topic
load awaits `F2`.
3. `R1`'s asynchronous close finally completes; its pending `readNextAsync`
fails with
`AlreadyClosedException`, and `readMorePoliciesAsync`'s AlreadyClosed
branch runs the
namespace-keyed cleanup, which removes `R2` and completes `F2`
exceptionally with *"…aborted because
the cached state was cleared"*. The awaiting topic load fails.
The init-*failure* path was already hardened with the identity-guarded
`cleanupFailedPolicyCacheInit`
(#26025); the reader-close path was the remaining namespace-keyed teardown
that could clobber a newer
generation.
### Modifications
- Thread the initialization future through `readMorePoliciesAsync(reader,
initFuture)`.
- Route its AlreadyClosed cleanup (and the service-closed cleanup) through
the existing identity-guarded
`cleanupFailedPolicyCacheInit(namespace, initFuture, true)` instead of the
namespace-keyed
`cleanPoliciesCacheInitMap`. It only tears down state that still belongs
to this initialization, so a
superseded reader's late close becomes a no-op, while a still-current
reader tears down its own
generation exactly as before. This is sound because a newer reader/init
generation can only be
installed after the previous one has been removed (readers and init
futures are installed and removed
as a pair), so guarding on the init future's identity correctly
distinguishes the two.
- Extend the `cleanupFailedPolicyCacheInit` javadoc to cover the
reader-closed-after-init case.
### Verifying this change
This change added tests and can be verified as follows:
- Added
`SystemTopicBasedTopicPoliciesServiceTest.testClosedSupersededReaderDoesNotAbortReloadedInit`,
a deterministic regression test: it parks the old reader's read loop on a
controllable future, installs
a newer reader + still-pending init future for the namespace, then
completes the old reader's read with
`AlreadyClosedException` and asserts the newer generation's init future is
not aborted and its reader
stays cached. It fails on the previous code (the reload's init future is
aborted) and passes with the
fix.
- Updated
`testPrepareInitPoliciesCacheAsyncThrowExceptionAfterCreateReader`: the
reader-close path now
routes through the identity-guarded cleanup, so its Mockito verify counts
change
(`cleanPoliciesCacheInitMap` 1→0, `cleanupFailedPolicyCacheInit` 1→2).
- The full `SystemTopicBasedTopicPoliciesServiceTest` class and
`AdminApi2Test.testGetInternalStatsWithProperties` pass; checkstyle and
spotless are clean.
### Does this pull request potentially affect one of the following parts:
- [ ] 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 automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
