You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@pulsar.apache.org by "dao-jun (via GitHub)" <gi...@apache.org> on 2026/07/22 18:40:27 UTC

[PR] Fix per ledger properties [pulsar]

dao-jun opened a new pull request, #26227:
URL: https://github.com/apache/pulsar/pull/26227

   ### Motivation
   
     When a ledger is closed (either because it reaches max entries or is manually closed), the `ledgerClosed()` method creates a `new LedgerInfo` object with only basic fields (ledgerId, entries, size, timestamp), which discards any previously set properties.
   
     This causes a bug where ledger properties are lost after ledger closure:
     1. User calls `asyncAddLedgerProperty(ledgerId, key, value)` to set a property on the current ledger
     2. The property can be read successfully immediately after setting
     3. When the ledger gets closed (e.g., reaches maxEntriesPerLedger), the `ledgerClosed()` method replaces the LedgerInfo with a new instance that has no properties
     4. Subsequent reads of the property return null, even though the property was successfully persisted to the metadata store
   
   ### Modifications
   
   1. Modified the `ManagedLedgerImpl.ledgerClosed()` method to preserve existing ledger properties when updating the LedgerInfo on ledger close.
   2. Update tests
   
   ### Verifying this change
   
   - [ ] Make sure that the change passes the CI checks.
   
   *(Please pick either of the following options)*
   
   This change is a trivial rework / code cleanup without any test coverage.
   
   *(or)*
   
   This change is already covered by existing tests, such as *(please describe tests)*.
   
   *(or)*
   
   This change added tests and can be verified as follows:
   
   *(example:)*
     - *Added integration tests for end-to-end deployment with large payloads (10MB)*
     - *Extended integration test for recovery after broker failure*
   
   ### Does this pull request potentially affect one of the following parts:
   
   <!-- DO NOT REMOVE THIS SECTION. CHECK THE PROPER BOX ONLY. -->
   
   *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 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: commits-unsubscribe@pulsar.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

Re: [PR] [fix][ml] Preserve ledger properties when closing ledger [pulsar]

Posted by "lhotari (via GitHub)" <gi...@apache.org>.
lhotari commented on PR #26227:
URL: https://github.com/apache/pulsar/pull/26227#issuecomment-5606546646

   This fix is also relevant to branch-4.0, even though that branch does not contain PIP-404 per-ledger properties. Its `LedgerInfo` already contains `offloadContext`, which is also discarded when closing a ledger rebuilds `LedgerInfo` from scratch. Copying the existing metadata before refreshing entries, size, and timestamp preserves that context too.
   
   The local branch-4.0 backport uses the older protobuf builder API (`oldInfo.toBuilder()` and `build()`) and adapts the regression coverage to verify that offload context survives ledger closure/rollover. Compilation and the focused regression tests passed.
   


-- 
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: commits-unsubscribe@pulsar.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

Re: [PR] [fix][ml] Preserve ledger properties when closing ledger [pulsar]

Posted by "lhotari (via GitHub)" <gi...@apache.org>.
lhotari commented on PR #26227:
URL: https://github.com/apache/pulsar/pull/26227#issuecomment-5078814778

   PIP-404 doesn't exist on branch-4.0


-- 
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: commits-unsubscribe@pulsar.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

Re: [PR] [fix][ml] Preserve ledger properties when closing ledger [pulsar]

Posted by "dao-jun (via GitHub)" <gi...@apache.org>.
dao-jun merged PR #26227:
URL: https://github.com/apache/pulsar/pull/26227


-- 
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: commits-unsubscribe@pulsar.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org