[fix][ml] Make the EntryImpl poison-position regression test actually recycle

[dao-jun]

Motivation

The post-merge review of #26707 (thanks @void-ptr974) pointed out that the regression test added there does not actually exercise recycling: on Netty 4.2 the Recycler only pools instances for FastThreadLocalThreads — a plain thread receives a no-op handle, so every get() allocates a fresh object. The merged test ran on an ordinary test worker thread and still passes with the three entry.position = null resets removed.

The review also correctly noted that the comment above the reset overclaims: a getPosition() concurrent with create() can still read the old ids and publish its stale value after the reset (unsynchronized fields, no generation check), so the reset is best-effort hardening for the sequential poison-and-reuse case — post-release access remains a caller bug to fix at the call site.

Modifications

• EntryImplTest.testRecycledObjectDoesNotInheritPoisonedPosition now runs the poison-and-reuse scenario on a FastThreadLocalThread and proves instance identity with assertSame, guarded by a bounded warm-up that fails the test if the per-thread pool never hands the same instance back (absorbs io.netty.recycler.ratio warm-up behavior).
• The scenario now covers all three lazy-position create(...) variants, including create(LedgerEntry, int) — the variant the managed-ledger read path actually uses.
• Reworded the comment above the reset in EntryImpl.create(LedgerEntry, int) to state the actual guarantee and its limits (sequential case only; concurrent post-release access is out of scope and must be fixed at the call site).

No production behavior change — the three reset lines are untouched.

Verifying this change

• ./gradlew :managed-ledger:test --tests "org.apache.bookkeeper.mledger.impl.EntryImplTest" — 12/12 green.
• Red/green verified: with the three entry.position = null resets temporarily removed, the new test fails (create(LedgerEntry, int): a recycled entry must not inherit the poisoned (-1, -1) position); restoring them is green.

This change is a trivial rework: no production logic is modified beyond a comment.

[utafrali]

The fix correctly addresses the original test's flaw: running on a FastThreadLocalThread, proving instance identity with assertSame, covering all three create variants including the production read-path variant, and using a bounded warm-up that fails loudly rather than silently passing on fresh allocations. The production comment is now accurate about sequential-case-only guarantees. Two minor suggestions: join the thread after the future resolves, and prefer assertEquals over assertTrue(...compareTo(...) == 0) for actionable failure messages.

[utafrali]

completion.get(30, TimeUnit.SECONDS) returns as soon as the future is resolved, but the thread is technically still alive until its run() returns. The gap is tiny, but adding thread.join() after completion.get(...) makes the lifecycle explicit and prevents any thread-leak warnings from strict test frameworks:

completion.get(30, TimeUnit.SECONDS);
thread.join();

[utafrali]

assertTrue(x.compareTo(...) == 0, msg) produces a failure message like expected true but got false. Swapping to assertEquals would print the actual and expected positions on failure, which is much more useful when debugging:

assertEquals(second.getPosition(), PositionFactory.create(expectedLedgerId, expectedEntryId),
        variant + ": a recycled entry must not inherit the poisoned (-1, -1) position");

The compareTo-based form is already used elsewhere in this file, so this isn't blocking, but it's worth changing in a test whose whole point is catching bad values.