[improve][ml] Migrate managed-ledger from protobuf to LightProto#25336
Open
merlimat wants to merge 9 commits intoapache:masterfrom
Open
[improve][ml] Migrate managed-ledger from protobuf to LightProto#25336merlimat wants to merge 9 commits intoapache:masterfrom
merlimat wants to merge 9 commits intoapache:masterfrom
Conversation
Migrate the managed-ledger module from Google protobuf to LightProto for ManagedLedgerInfo and ManagedCursorInfo proto messages. Key changes: - Replace protobuf Builder pattern with direct LightProto mutable objects - Replace ByteString with byte[] for bytes fields - Replace static parseFrom() with instance parseFrom(ByteBuf, size) - Reuse LightProto objects in ManagedCursorImpl and MetaStoreImpl to reduce allocations on hot paths - Use indexed loops (getXxxCount() + getXxxAt(i)) for repeated fields instead of getXxxList() to avoid list copy overhead - Update all test files to use LightProto API
merlimat
force-pushed
the
lp-managed-ledger-reuse
branch
from
6e4b8e8 to
add770a
Compare
…-ledger proto types Update broker test files that referenced the removed MLDataFormats outer class wrapper. Replace MLDataFormats.ManagedLedgerInfo with ManagedLedgerInfo and convert LedgerInfo.newBuilder().build() to new LedgerInfo() constructor pattern.
LightProto 0.6.2 fixes optional field behavior to return default values instead of throwing exceptions, matching standard protobuf behavior.
…tProto The test was using reflection to call a private method with a List parameter that no longer exists. Updated to use the public recoverIndividualDeletedMessages(PositionInfo) method directly.
LightProto does not implement equals(), so compare serialized bytes instead of object equality.
Compare serialized bytes instead of object equality since LightProto does not implement equals(). Also call materialize() after parsing from ByteBuf to ensure the data is fully deserialized before releasing the buffer.
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## master #25336 +/- ##
=============================================
+ Coverage 37.37% 72.70% +35.32%
+ Complexity 13188 2460 -10728
=============================================
Files 1897 1954 +57
Lines 150557 154775 +4218
Branches 17215 17720 +505
=============================================
+ Hits 56276 112530 +56254
+ Misses 86612 33213 -53399
- Partials 7669 9032 +1363
Flags with carried forward coverage won't be shown. Click here to find out more.
🚀 New features to boost your workflow:
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Motivation
Migrate the managed-ledger module from Google protobuf to LightProto for
ManagedLedgerInfoandManagedCursorInfoproto messages. LightProto generatesmutable, allocation-friendly Java classes with the same wire format, reducing GC
pressure on hot paths.
Modifications
ByteStringwithbyte[]for bytes fieldsparseFrom()with instanceparseFrom(ByteBuf, size)ManagedCursorImplandMetaStoreImpltoreduce allocations on hot paths
getXxxCount()+getXxxAt(i)) for repeated fieldsinstead of
getXxxList()to avoid list copy overheadDocumentation
doc-not-neededMatching PR in forked repository
No response
Note
This PR depends on #25332 (Upgrade LightProto to 0.6.1)
ready-to-test