[improve][meta] Support set metadata size threshold for compression.#19561
Conversation
if metadata size before serialization is smaller then threshold. compression will not apply.
|
@codelipenghui @Technoboy- @AnonHxy plesase take a look. thank you. |
| * ManagedLedgerInfo compression threshold. If the origin metadata size below configuration. | ||
| * compression will not apply. | ||
| */ | ||
| private int managedLedgerInfoCompressionSizeThresholdBytes = 0; |
There was a problem hiding this comment.
Should we set a suitable default value?
There was a problem hiding this comment.
current value is the same behavior before this feature. maybe we can set to 128 or more. In our production env most cursor metadata is less than this value.
|
/pulsarbot run-failure-checks |
|
@Technoboy- @congbobo184 @poorbarcode can you take a look ？ |
poorbarcode
left a comment
There was a problem hiding this comment.
LGTM. Left some comments
|
@poorbarcode @Technoboy- @315157973 can this pr be merged. if need change please let me know : - ) |
|
Some unit tests fail, please rebase master |
Sorry for the late response. |
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## master #19561 +/- ##
=============================================
+ Coverage 30.23% 72.96% +42.72%
- Complexity 295 31913 +31618
=============================================
Files 1689 1865 +176
Lines 129020 138445 +9425
Branches 14070 15192 +1122
=============================================
+ Hits 39013 101015 +62002
+ Misses 84148 29436 -54712
- Partials 5859 7994 +2135
Flags with carried forward coverage won't be shown. Click here to find out more.
🚀 New features to boost your workflow:
|
|
@lifepuzzlefun Please fix the unit test |
4282e6d to
2d52942
Compare
|
I found one flaky test in previous CI and try to fix by #19793. the current fail in https://github.com/apache/pulsar/actions/runs/4382425219/jobs/7688447199
trigger another CI run to see if it fail again. |
|
@315157973 @poorbarcode @Technoboy- can this pr be merged？ |
The button is unavailable due to unit tests not all passing |
|
@congbobo184 aha, i forget add those config in the pulsar configuration. i have add them in the new commit : - ) |
|
@lifepuzzlefun hi, add any config, we also should create a PIP to discuss and vote |
When read from schemaCache, check if the schema version means empty. when use Schema.BYTES consumer and enable retry and dlq policy. need cache the empty schema version to avoid send a lot of GetOrCreateSchema request to retrieve the schema version.
|
The pr had no activity for 30 days, mark with Stale label. |
|
@congbobo184 hi, sorry for the late reply, i have just create a pip to discuss. |
|
#20307 pip-270 created. hopes for your comments and vote, thank you~ the discuss mail list address is write at the bottom, feel free to join the discuss. |
|
@congbobo184 Could you take a review? |
PIP-270
Motivation
Current we support to set compression on ManagedLedger and ManagedCursor metadata
by managedLedgerInfoCompressionType and managedCursorInfoCompressionType
the metadata of ManagedLedger is meanly composed by a ledger id list and some other field
to describe pulsar topic storage state. you can use the metadata to find which the message is stored.
when pulsar topic message is deleled by the retention policy or ledger rollover. the metadata is remove or append the metadata list.
this metadata can be small when retention time is short.
the metadata of ManagedCursor is the subscription state of a pulsar subscription. if user ack an message this will trigger the subscription state save in the metadata of ManagedCursor
if user always use accumlateAcknowledge api this state is also small enough.
all those metadata is saved in metadataStore.
sometimes the metadata is too small to compress. so a size based config is needed.
Modifications
check size before metadata compression.
Verifying this change
This change added tests and can be verified as follows:
add unit test case for metadata big or small than threshold.
Does this pull request potentially affect one of the following parts:
If the box was checked, please highlight the changes
Documentation
docdoc-requireddoc-not-neededdoc-completeMatching PR in forked repository
PR in forked repository: