You are viewing a plain text version of this content. The canonical link for it is here.

Posted to commits@pulsar.apache.org by GitBox <gi...@apache.org> on 2021/11/10 07:49:33 UTC

[GitHub] [pulsar] BewareMyPower opened a new pull request #12714: [Managed Ledger] Fix the incorrect total size when BrokerEntryMetadata is enabled

BewareMyPower opened a new pull request #12714:
URL: https://github.com/apache/pulsar/pull/12714

### Motivation

When the BrokerEntryMetadata is enabled, the total size in `ManagedLedgerImpl` is inaccurate. Because when the total size is updated in `OpAddEntry#safeRun`, the `dataLength` is the initial size of `data` when `OpAddEntry` is constructed, but `data` could be changed via `setData` method.

The inaccurate total size could affect the retention size validation. Because in `ManagedLedgerImpl#internalTrimLedgers`, the total size reduces by the size of `LedgerInfo`, which is assigned from the `LedgerHandle#getLength()`. Therefore, the total size will become 0 or less before all ledgers are removed.

### Modifications

- Remove the `dataLength` field from `OpAddEntry`. Instead, add a `getDataLength()` method to retrieve the latest dat size.
- Add a `testManagedLedgerTotalSize` test to `BrokerEntryMetadataE2ETest`. It produces 10 messages and trigger the rollover manually so that the first `LedgerInfo` of the managed ledger contains the correct total bytes. Then compare the `totalSize` field with it to verify this fix works.

### Verifying this change

- [ ] Make sure that the change passes the CI checks.

This change added test `BrokerEntryMetadataE2ETest#testManagedLedgerTotalSize`.

--
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

[GitHub] [pulsar] codelipenghui commented on pull request #12714: [Managed Ledger] Fix the incorrect total size when BrokerEntryMetadata is enabled

Posted by GitBox <gi...@apache.org>.

codelipenghui commented on pull request #12714:
URL: https://github.com/apache/pulsar/pull/12714#issuecomment-964877552


   @BewareMyPower 
   
   > but data could be changed via setData method.
   
   Can we update the dataLength when calling setData()? We might get different value by readableBytes() which there is a reader read the data from the buffer. 


-- 
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

[GitHub] [pulsar] eolivelli commented on pull request #12714: [Managed Ledger] Fix the incorrect total size when BrokerEntryMetadata is enabled

Posted by GitBox <gi...@apache.org>.

eolivelli commented on pull request #12714:
URL: https://github.com/apache/pulsar/pull/12714#issuecomment-964889658


   > @BewareMyPower
   > 
   > > but data could be changed via setData method.
   > 
   > Can we update the dataLength when calling setData()? We might get different value by readableBytes() which there is a reader read the data from the buffer.
   
   +1


-- 
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

[GitHub] [pulsar] BewareMyPower commented on pull request #12714: [Managed Ledger] Fix the incorrect total size when BrokerEntryMetadata is enabled

Posted by GitBox <gi...@apache.org>.

BewareMyPower commented on pull request #12714:
URL: https://github.com/apache/pulsar/pull/12714#issuecomment-964904526


   @codelipenghui @eolivelli done.


-- 
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

[GitHub] [pulsar] BewareMyPower merged pull request #12714: [Managed Ledger] Fix the incorrect total size when BrokerEntryMetadata is enabled

Posted by GitBox <gi...@apache.org>.

BewareMyPower merged pull request #12714:
URL: https://github.com/apache/pulsar/pull/12714


   


-- 
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