apache / apache/pulsar

Eliminate special process logic for ManagedLedgerImpl

Open
#25,019 4 comments 0 reactions 0 assignees View on GitHub
type/enhancement
Dominant language
Java
Stars
15.3k
Forks
3.8k
Avg merge
1d 14h
Merged PRs (30d)
160

Description

### Search before reporting

- [x] I searched in the [issues](https://github.com/apache/pulsar/issues) and found nothing similar.

### Motivation

See https://github.com/apache/pulsar/pull/25016#discussion_r2560288325

Since https://github.com/apache/pulsar/pull/24622, when a managed ledger does not inherit `ManagedLedgerImpl`, the expiration check will use a different method, which is hard to test without a custom implementation.

```java
if (managedLedger instanceof ManagedLedgerImpl ml) {
checkMessageExpiryWithSharedPosition(ml, messageTtlInSeconds);
} else {
// Fallback to the slower solution if managed ledger is not an instance of ManagedLedgerImpl: each
// subscription find position and handle expiring itself.
checkMessageExpiryWithoutSharedPosition(messageTtlInSeconds);
}
```

However, in `checkMessageExpiryWithSharedPosition`, the only operation is getting the cursor with oldest position:

```java
private void checkMessageExpiryWithSharedPosition(ManagedLedgerImpl ml, int messageTtlInSeconds) {
// Find the target position at one time, then expire all subscriptions and replicators.
final var cursorWithOldestPosition = ml.getCursors().getCursorWithOldestPosition();
```

This should be a general operation that if the `ManagedLedger#getCursors` returns a correct cursor container, it should also work even if it's not a `ManagedLedgerImpl`.

### Solution

_No response_

### Alternatives

_No response_

### Anything else?

_No response_

### Are you willing to submit a PR?

- [ ] I'm willing to submit a PR!

Contributor guide

Open the contributing guide

Research direction

Start with the expiration-check logic in ManagedLedgerImpl, especially checkMessageExpiryWithSharedPosition and checkMessageExpiryWithoutSharedPosition, and review the discussion in PR 25016. Trace the ManagedLedger#getCursors contract and cursor-with-oldest-position operation. Done means expiration handling no longer depends on an instanceof ManagedLedgerImpl and remains testable with another managed-ledger implementation.

Written by the indexing model from the issue text.

Assessment

Tech stack
java
Domain
distributed-systems
Issue type
Refactor
Difficulty
4/5
Estimated time
3-5 days
Activity status
Quiet
Clarity
Mostly clear
Newbie friendliness
55/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.