You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@pulsar.apache.org by "lifepuzzlefun (via GitHub)" <gi...@apache.org> on 2023/03/09 11:51:59 UTC
[GitHub] [pulsar] lifepuzzlefun opened a new pull request, #19561: [improve][meta] Support set metadata size threshold for compression.
lifepuzzlefun opened a new pull request, #19561:
URL: https://github.com/apache/pulsar/pull/19561
Fixes #xyz
### Motivation
sometime metadata is too small for compression. add size threshold to check if compress metadata.
if metadata size before serialization is smaller then threshold. compression will not apply.
### 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:
<!-- 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
### Documentation
<!-- DO NOT REMOVE THIS SECTION. CHECK THE PROPER BOX ONLY. -->
- [ ] `doc` <!-- Your PR contains doc changes. -->
- [ ] `doc-required` <!-- Your PR changes impact docs and you will update later -->
- [x] `doc-not-needed` <!-- Your PR changes do not impact docs -->
- [ ] `doc-complete` <!-- Docs have been already added -->
### Matching PR in forked repository
PR in forked repository: <!-- ENTER URL HERE -->
--
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] 315157973 commented on pull request #19561: [improve][meta] Support set metadata size threshold for compression.
Posted by "315157973 (via GitHub)" <gi...@apache.org>.
315157973 commented on PR #19561:
URL: https://github.com/apache/pulsar/pull/19561#issuecomment-1461584704
Some unit tests fail, please rebase master
--
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] lifepuzzlefun commented on pull request #19561: [improve][meta] Support set metadata size threshold for compression.
Posted by "lifepuzzlefun (via GitHub)" <gi...@apache.org>.
lifepuzzlefun commented on PR #19561:
URL: https://github.com/apache/pulsar/pull/19561#issuecomment-1544998480
@congbobo184 hi, sorry for the late reply, i have just create a pip to discuss.
--
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] codecov-commenter commented on pull request #19561: [improve][meta] Support set metadata size threshold for compression.
Posted by "codecov-commenter (via GitHub)" <gi...@apache.org>.
codecov-commenter commented on PR #19561:
URL: https://github.com/apache/pulsar/pull/19561#issuecomment-1463476049
## [Codecov](https://codecov.io/gh/apache/pulsar/pull/19561?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) Report
> Merging [#19561](https://codecov.io/gh/apache/pulsar/pull/19561?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (4282e6d) into [master](https://codecov.io/gh/apache/pulsar/commit/e0b50c9ec5f12d0fb8275f235d8ac00e87a9099e?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (e0b50c9) will **increase** coverage by `6.64%`.
> The diff coverage is `50.00%`.
[](https://codecov.io/gh/apache/pulsar/pull/19561?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
```diff
@@ Coverage Diff @@
## master #19561 +/- ##
============================================
+ Coverage 24.65% 31.30% +6.64%
- Complexity 291 293 +2
============================================
Files 1591 1673 +82
Lines 123073 131631 +8558
Branches 13434 15117 +1683
============================================
+ Hits 30348 41209 +10861
+ Misses 88233 84320 -3913
- Partials 4492 6102 +1610
```
| Flag | Coverage Δ | |
|---|---|---|
| inttests | `25.53% <50.00%> (+0.88%)` | :arrow_up: |
| systests | `25.34% <50.00%> (?)` | |
Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#carryforward-flags-in-the-pull-request-comment) to find out more.
| [Impacted Files](https://codecov.io/gh/apache/pulsar/pull/19561?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | Coverage Δ | |
|---|---|---|
| [.../apache/bookkeeper/mledger/impl/MetaStoreImpl.java](https://codecov.io/gh/apache/pulsar/pull/19561?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-bWFuYWdlZC1sZWRnZXIvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2Jvb2trZWVwZXIvbWxlZGdlci9pbXBsL01ldGFTdG9yZUltcGwuamF2YQ==) | `43.16% <18.51%> (+2.15%)` | :arrow_up: |
| [.../bookkeeper/mledger/MetadataCompressionConfig.java](https://codecov.io/gh/apache/pulsar/pull/19561?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-bWFuYWdlZC1sZWRnZXIvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2Jvb2trZWVwZXIvbWxlZGdlci9NZXRhZGF0YUNvbXByZXNzaW9uQ29uZmlnLmphdmE=) | `60.00% <60.00%> (ø)` | |
| [...bookkeeper/mledger/ManagedLedgerFactoryConfig.java](https://codecov.io/gh/apache/pulsar/pull/19561?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-bWFuYWdlZC1sZWRnZXIvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2Jvb2trZWVwZXIvbWxlZGdlci9NYW5hZ2VkTGVkZ2VyRmFjdG9yeUNvbmZpZy5qYXZh) | `90.00% <100.00%> (+2.50%)` | :arrow_up: |
| [...kkeeper/mledger/impl/ManagedLedgerFactoryImpl.java](https://codecov.io/gh/apache/pulsar/pull/19561?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-bWFuYWdlZC1sZWRnZXIvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2Jvb2trZWVwZXIvbWxlZGdlci9pbXBsL01hbmFnZWRMZWRnZXJGYWN0b3J5SW1wbC5qYXZh) | `40.87% <100.00%> (+0.11%)` | :arrow_up: |
| [...org/apache/pulsar/broker/ServiceConfiguration.java](https://codecov.io/gh/apache/pulsar/pull/19561?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cHVsc2FyLWJyb2tlci1jb21tb24vc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL3B1bHNhci9icm9rZXIvU2VydmljZUNvbmZpZ3VyYXRpb24uamF2YQ==) | `98.00% <100.00%> (+0.12%)` | :arrow_up: |
| [...ache/pulsar/broker/ManagedLedgerClientFactory.java](https://codecov.io/gh/apache/pulsar/pull/19561?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cHVsc2FyLWJyb2tlci9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcHVsc2FyL2Jyb2tlci9NYW5hZ2VkTGVkZ2VyQ2xpZW50RmFjdG9yeS5qYXZh) | `46.15% <100.00%> (+2.91%)` | :arrow_up: |
... and [540 files with indirect coverage changes](https://codecov.io/gh/apache/pulsar/pull/19561/indirect-changes?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
--
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] congbobo184 commented on pull request #19561: [improve][meta] Support set metadata size threshold for compression.
Posted by "congbobo184 (via GitHub)" <gi...@apache.org>.
congbobo184 commented on PR #19561:
URL: https://github.com/apache/pulsar/pull/19561#issuecomment-1475517655
@lifepuzzlefun hi, add any config, we also should create a PIP to discuss and vote
--
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] 315157973 commented on pull request #19561: [improve][meta] Support set metadata size threshold for compression.
Posted by "315157973 (via GitHub)" <gi...@apache.org>.
315157973 commented on PR #19561:
URL: https://github.com/apache/pulsar/pull/19561#issuecomment-1447635433
/pulsarbot run-failure-checks
--
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] github-actions[bot] commented on pull request #19561: [improve][meta] Support set metadata size threshold for compression.
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #19561:
URL: https://github.com/apache/pulsar/pull/19561#issuecomment-1543176804
The pr had no activity for 30 days, mark with Stale label.
--
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] Technoboy- commented on pull request #19561: [improve][meta] Support set metadata size threshold for compression.
Posted by "Technoboy- (via GitHub)" <gi...@apache.org>.
Technoboy- commented on PR #19561:
URL: https://github.com/apache/pulsar/pull/19561#issuecomment-1566426516
@congbobo184 Could you take a review?
--
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] lifepuzzlefun commented on pull request #19561: [improve][meta] Support set metadata size threshold for compression.
Posted by "lifepuzzlefun (via GitHub)" <gi...@apache.org>.
lifepuzzlefun commented on PR #19561:
URL: https://github.com/apache/pulsar/pull/19561#issuecomment-1469596475
@315157973 @poorbarcode @Technoboy- can this pr be merged？
--
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] Technoboy- commented on pull request #19561: [improve][meta] Support set metadata size threshold for compression.
Posted by "Technoboy- (via GitHub)" <gi...@apache.org>.
Technoboy- commented on PR #19561:
URL: https://github.com/apache/pulsar/pull/19561#issuecomment-1461902406
> @Technoboy- @congbobo184 @poorbarcode can you take a look ？
Sorry for the late response.
--
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] lifepuzzlefun commented on pull request #19561: [improve][meta] Support set metadata size threshold for compression.
Posted by "lifepuzzlefun (via GitHub)" <gi...@apache.org>.
lifepuzzlefun commented on PR #19561:
URL: https://github.com/apache/pulsar/pull/19561#issuecomment-1437874705
@codelipenghui @Technoboy- @AnonHxy plesase take a look. thank you.
--
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] 315157973 commented on a diff in pull request #19561: [improve][meta] Support set metadata size threshold for compression.
Posted by "315157973 (via GitHub)" <gi...@apache.org>.
315157973 commented on code in PR #19561:
URL: https://github.com/apache/pulsar/pull/19561#discussion_r1113015289
##########
managed-ledger/src/main/java/org/apache/bookkeeper/mledger/ManagedLedgerFactoryConfig.java:
##########
@@ -92,8 +92,30 @@ public class ManagedLedgerFactoryConfig {
*/
private String managedLedgerInfoCompressionType = MLDataFormats.CompressionType.NONE.name();
+ /**
+ * ManagedLedgerInfo compression threshold. If the origin metadata size below configuration.
+ * compression will not apply.
+ */
+ private int managedLedgerInfoCompressionSizeThresholdBytes = 0;
Review Comment:
Should we set a suitable default value?
--
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] lifepuzzlefun closed pull request #19561: [improve][meta] Support set metadata size threshold for compression.
Posted by "lifepuzzlefun (via GitHub)" <gi...@apache.org>.
lifepuzzlefun closed pull request #19561: [improve][meta] Support set metadata size threshold for compression.
URL: https://github.com/apache/pulsar/pull/19561
--
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] lifepuzzlefun commented on pull request #19561: [improve][meta] Support set metadata size threshold for compression.
Posted by "lifepuzzlefun (via GitHub)" <gi...@apache.org>.
lifepuzzlefun commented on PR #19561:
URL: https://github.com/apache/pulsar/pull/19561#issuecomment-1474664693
@congbobo184 aha, i forget add those config in the pulsar configuration. i have add them in the new commit : - )
and do we need a pip to merge this patch ? this is a small feature introduce to pulsar. may be i can update the doc to tell people the support settings.
--
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] 315157973 closed pull request #19561: [improve][meta] Support set metadata size threshold for compression.
Posted by "315157973 (via GitHub)" <gi...@apache.org>.
315157973 closed pull request #19561: [improve][meta] Support set metadata size threshold for compression.
URL: https://github.com/apache/pulsar/pull/19561
--
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] AnonHxy commented on a diff in pull request #19561: [improve][meta] Support set metadata size threshold for compression.
Posted by "AnonHxy (via GitHub)" <gi...@apache.org>.
AnonHxy commented on code in PR #19561:
URL: https://github.com/apache/pulsar/pull/19561#discussion_r1112577657
##########
managed-ledger/src/main/java/org/apache/bookkeeper/mledger/ManagedLedgerFactoryConfig.java:
##########
@@ -92,8 +92,30 @@ public class ManagedLedgerFactoryConfig {
*/
private String managedLedgerInfoCompressionType = MLDataFormats.CompressionType.NONE.name();
+ /**
+ * ManagedLedgerInfo compression threshold. If the origin metadata size below configuration.
+ * compression will not apply.
+ */
+ private int managedLedgerInfoCompressionSizeThresholdBytes = 0;
+
/**
* ManagedCursorInfo compression type. If the compression type is null or invalid, don't compress data.
*/
private String managedCursorInfoCompressionType = MLDataFormats.CompressionType.NONE.name();
+
+ /**
+ * ManagedCursorInfo compression threshold. If the origin metadata size below configuration.
+ * compression will not apply.
+ */
+ private int managedCursorInfoCompressionSizeThresholdBytes = 0;
+
+ public MetadataCompressionConfig getCompressionConfigForManagedLedgerInfo() {
Review Comment:
There is no need to add getter method here
--
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] lifepuzzlefun commented on pull request #19561: [improve][meta] Support set metadata size threshold for compression.
Posted by "lifepuzzlefun (via GitHub)" <gi...@apache.org>.
lifepuzzlefun commented on PR #19561:
URL: https://github.com/apache/pulsar/pull/19561#issuecomment-1449662576
@Technoboy- @congbobo184 @poorbarcode can you take a look ？
--
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] 315157973 commented on pull request #19561: [improve][meta] Support set metadata size threshold for compression.
Posted by "315157973 (via GitHub)" <gi...@apache.org>.
315157973 commented on PR #19561:
URL: https://github.com/apache/pulsar/pull/19561#issuecomment-1464814475
@lifepuzzlefun Please fix the unit test
--
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] lifepuzzlefun commented on a diff in pull request #19561: [improve][meta] Support set metadata size threshold for compression.
Posted by "lifepuzzlefun (via GitHub)" <gi...@apache.org>.
lifepuzzlefun commented on code in PR #19561:
URL: https://github.com/apache/pulsar/pull/19561#discussion_r1113775318
##########
managed-ledger/src/main/java/org/apache/bookkeeper/mledger/ManagedLedgerFactoryConfig.java:
##########
@@ -92,8 +92,30 @@ public class ManagedLedgerFactoryConfig {
*/
private String managedLedgerInfoCompressionType = MLDataFormats.CompressionType.NONE.name();
+ /**
+ * ManagedLedgerInfo compression threshold. If the origin metadata size below configuration.
+ * compression will not apply.
+ */
+ private int managedLedgerInfoCompressionSizeThresholdBytes = 0;
Review 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.
--
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] poorbarcode commented on a diff in pull request #19561: [improve][meta] Support set metadata size threshold for compression.
Posted by "poorbarcode (via GitHub)" <gi...@apache.org>.
poorbarcode commented on code in PR #19561:
URL: https://github.com/apache/pulsar/pull/19561#discussion_r1121698204
##########
managed-ledger/src/main/java/org/apache/bookkeeper/mledger/MetadataCompressionConfig.java:
##########
@@ -0,0 +1,56 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.bookkeeper.mledger;
+
+import lombok.AllArgsConstructor;
+import lombok.Data;
+import lombok.ToString;
+import org.apache.bookkeeper.mledger.proto.MLDataFormats;
+import org.apache.commons.lang.StringUtils;
+
+@Data
+@AllArgsConstructor
+@ToString
+public class MetadataCompressionConfig {
+ MLDataFormats.CompressionType compressionType;
+ int compressThreshold;
+
+ public MetadataCompressionConfig(String compressionType) throws IllegalArgumentException {
+ this(compressionType, 0);
+ }
+
+ public MetadataCompressionConfig(String compressionType, int compressThreshold) throws IllegalArgumentException {
+ this.compressionType = parseCompressionType(compressionType);
+ this.compressThreshold = compressThreshold;
+ }
+
+ public static MetadataCompressionConfig noCompression =
+ new MetadataCompressionConfig(MLDataFormats.CompressionType.NONE, 0);
Review Comment:
This variable should be final, right?
##########
managed-ledger/src/main/java/org/apache/bookkeeper/mledger/MetadataCompressionConfig.java:
##########
@@ -0,0 +1,56 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.bookkeeper.mledger;
+
+import lombok.AllArgsConstructor;
+import lombok.Data;
+import lombok.ToString;
+import org.apache.bookkeeper.mledger.proto.MLDataFormats;
+import org.apache.commons.lang.StringUtils;
+
+@Data
+@AllArgsConstructor
+@ToString
+public class MetadataCompressionConfig {
+ MLDataFormats.CompressionType compressionType;
+ int compressThreshold;
Review Comment:
These two variables will not change, right? If yes, please append the modifier `final` and remove `@Data` above the class
--
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] Technoboy- closed pull request #19561: [improve][meta] Support set metadata size threshold for compression.
Posted by "Technoboy- (via GitHub)" <gi...@apache.org>.
Technoboy- closed pull request #19561: [improve][meta] Support set metadata size threshold for compression.
URL: https://github.com/apache/pulsar/pull/19561
--
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] lifepuzzlefun commented on pull request #19561: [improve][meta] Support set metadata size threshold for compression.
Posted by "lifepuzzlefun (via GitHub)" <gi...@apache.org>.
lifepuzzlefun commented on PR #19561:
URL: https://github.com/apache/pulsar/pull/19561#issuecomment-1464908991
I found one flaky test in previous CI and try to fix by https://github.com/apache/pulsar/pull/19793.
the current fail in https://github.com/apache/pulsar/actions/runs/4382425219/jobs/7688447199
`org.apache.pulsar.broker.service.ManagedLedgerCompressionTest.testRestartBrokerEnableManagedLedgerInfoCompression(org.apache.pulsar.broker.service.ManagedLedgerCompressionTest)` pass in my local env.
trigger another CI run to see if it fail again.
--
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] Technoboy- merged pull request #19561: [improve][meta] Support set metadata size threshold for compression.
Posted by "Technoboy- (via GitHub)" <gi...@apache.org>.
Technoboy- merged PR #19561:
URL: https://github.com/apache/pulsar/pull/19561
--
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] lifepuzzlefun commented on pull request #19561: [improve][meta] Support set metadata size threshold for compression.
Posted by "lifepuzzlefun (via GitHub)" <gi...@apache.org>.
lifepuzzlefun commented on PR #19561:
URL: https://github.com/apache/pulsar/pull/19561#issuecomment-1545000232
https://github.com/apache/pulsar/issues/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.
--
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] lifepuzzlefun commented on a diff in pull request #19561: [improve][meta] Support set metadata size threshold for compression.
Posted by "lifepuzzlefun (via GitHub)" <gi...@apache.org>.
lifepuzzlefun commented on code in PR #19561:
URL: https://github.com/apache/pulsar/pull/19561#discussion_r1124177203
##########
managed-ledger/src/main/java/org/apache/bookkeeper/mledger/MetadataCompressionConfig.java:
##########
@@ -0,0 +1,56 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.bookkeeper.mledger;
+
+import lombok.AllArgsConstructor;
+import lombok.Data;
+import lombok.ToString;
+import org.apache.bookkeeper.mledger.proto.MLDataFormats;
+import org.apache.commons.lang.StringUtils;
+
+@Data
+@AllArgsConstructor
+@ToString
+public class MetadataCompressionConfig {
+ MLDataFormats.CompressionType compressionType;
+ int compressThreshold;
+
+ public MetadataCompressionConfig(String compressionType) throws IllegalArgumentException {
+ this(compressionType, 0);
+ }
+
+ public MetadataCompressionConfig(String compressionType, int compressThreshold) throws IllegalArgumentException {
+ this.compressionType = parseCompressionType(compressionType);
+ this.compressThreshold = compressThreshold;
+ }
+
+ public static MetadataCompressionConfig noCompression =
+ new MetadataCompressionConfig(MLDataFormats.CompressionType.NONE, 0);
Review Comment:
done
##########
managed-ledger/src/main/java/org/apache/bookkeeper/mledger/MetadataCompressionConfig.java:
##########
@@ -0,0 +1,56 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.bookkeeper.mledger;
+
+import lombok.AllArgsConstructor;
+import lombok.Data;
+import lombok.ToString;
+import org.apache.bookkeeper.mledger.proto.MLDataFormats;
+import org.apache.commons.lang.StringUtils;
+
+@Data
+@AllArgsConstructor
+@ToString
+public class MetadataCompressionConfig {
+ MLDataFormats.CompressionType compressionType;
+ int compressThreshold;
Review Comment:
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] lifepuzzlefun commented on a diff in pull request #19561: [improve][meta] Support set metadata size threshold for compression.
Posted by "lifepuzzlefun (via GitHub)" <gi...@apache.org>.
lifepuzzlefun commented on code in PR #19561:
URL: https://github.com/apache/pulsar/pull/19561#discussion_r1113770434
##########
managed-ledger/src/main/java/org/apache/bookkeeper/mledger/ManagedLedgerFactoryConfig.java:
##########
@@ -92,8 +92,30 @@ public class ManagedLedgerFactoryConfig {
*/
private String managedLedgerInfoCompressionType = MLDataFormats.CompressionType.NONE.name();
+ /**
+ * ManagedLedgerInfo compression threshold. If the origin metadata size below configuration.
+ * compression will not apply.
+ */
+ private int managedLedgerInfoCompressionSizeThresholdBytes = 0;
+
/**
* ManagedCursorInfo compression type. If the compression type is null or invalid, don't compress data.
*/
private String managedCursorInfoCompressionType = MLDataFormats.CompressionType.NONE.name();
+
+ /**
+ * ManagedCursorInfo compression threshold. If the origin metadata size below configuration.
+ * compression will not apply.
+ */
+ private int managedCursorInfoCompressionSizeThresholdBytes = 0;
+
+ public MetadataCompressionConfig getCompressionConfigForManagedLedgerInfo() {
Review Comment:
It is for create MetadataCompressionConfig which encapsulate the compression type and threshold。
##########
managed-ledger/src/main/java/org/apache/bookkeeper/mledger/ManagedLedgerFactoryConfig.java:
##########
@@ -92,8 +92,30 @@ public class ManagedLedgerFactoryConfig {
*/
private String managedLedgerInfoCompressionType = MLDataFormats.CompressionType.NONE.name();
+ /**
+ * ManagedLedgerInfo compression threshold. If the origin metadata size below configuration.
+ * compression will not apply.
+ */
+ private int managedLedgerInfoCompressionSizeThresholdBytes = 0;
+
/**
* ManagedCursorInfo compression type. If the compression type is null or invalid, don't compress data.
*/
private String managedCursorInfoCompressionType = MLDataFormats.CompressionType.NONE.name();
+
+ /**
+ * ManagedCursorInfo compression threshold. If the origin metadata size below configuration.
+ * compression will not apply.
+ */
+ private int managedCursorInfoCompressionSizeThresholdBytes = 0;
+
+ public MetadataCompressionConfig getCompressionConfigForManagedLedgerInfo() {
Review Comment:
It is for create MetadataCompressionConfig which encapsulate the compression type and threshold.
--
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] lifepuzzlefun commented on a diff in pull request #19561: [improve][meta] Support set metadata size threshold for compression.
Posted by "lifepuzzlefun (via GitHub)" <gi...@apache.org>.
lifepuzzlefun commented on code in PR #19561:
URL: https://github.com/apache/pulsar/pull/19561#discussion_r1113777494
##########
managed-ledger/src/main/java/org/apache/bookkeeper/mledger/impl/MetaStoreImpl.java:
##########
@@ -415,29 +400,43 @@ private static MetaStoreException getException(Throwable t) {
}
public byte[] compressLedgerInfo(ManagedLedgerInfo managedLedgerInfo) {
- if (ledgerInfoCompressionType.equals(CompressionType.NONE)) {
+ CompressionType compressionType = ledgerInfoCompressionConfig.getCompressionType();
+ if (compressionType.equals(CompressionType.NONE)) {
return managedLedgerInfo.toByteArray();
}
- MLDataFormats.ManagedLedgerInfoMetadata mlInfoMetadata = MLDataFormats.ManagedLedgerInfoMetadata
- .newBuilder()
- .setCompressionType(ledgerInfoCompressionType)
- .setUncompressedSize(managedLedgerInfo.getSerializedSize())
- .build();
- return compressManagedInfo(managedLedgerInfo.toByteArray(), mlInfoMetadata.toByteArray(),
- mlInfoMetadata.getSerializedSize(), ledgerInfoCompressionType);
+
+ int uncompressedSize = managedLedgerInfo.getSerializedSize();
+ if (uncompressedSize > ledgerInfoCompressionConfig.getCompressThreshold()) {
+ MLDataFormats.ManagedLedgerInfoMetadata mlInfoMetadata = MLDataFormats.ManagedLedgerInfoMetadata
+ .newBuilder()
+ .setCompressionType(compressionType)
+ .setUncompressedSize(uncompressedSize)
+ .build();
+ return compressManagedInfo(managedLedgerInfo.toByteArray(), mlInfoMetadata.toByteArray(),
+ mlInfoMetadata.getSerializedSize(), compressionType);
+ }
+
+ return managedLedgerInfo.toByteArray();
}
public byte[] compressCursorInfo(ManagedCursorInfo managedCursorInfo) {
- if (cursorInfoCompressionType.equals(CompressionType.NONE)) {
+ CompressionType compressionType = cursorInfoCompressionConfig.getCompressionType();
+ if (compressionType.equals(CompressionType.NONE)) {
return managedCursorInfo.toByteArray();
}
- MLDataFormats.ManagedCursorInfoMetadata metadata = MLDataFormats.ManagedCursorInfoMetadata
- .newBuilder()
- .setCompressionType(cursorInfoCompressionType)
- .setUncompressedSize(managedCursorInfo.getSerializedSize())
- .build();
- return compressManagedInfo(managedCursorInfo.toByteArray(), metadata.toByteArray(),
- metadata.getSerializedSize(), cursorInfoCompressionType);
+
+ int uncompressedSize = managedCursorInfo.getSerializedSize();
+ if (uncompressedSize > cursorInfoCompressionConfig.getCompressThreshold()) {
+ MLDataFormats.ManagedCursorInfoMetadata metadata = MLDataFormats.ManagedCursorInfoMetadata
+ .newBuilder()
+ .setCompressionType(compressionType)
+ .setUncompressedSize(uncompressedSize)
+ .build();
+ return compressManagedInfo(managedCursorInfo.toByteArray(), metadata.toByteArray(),
+ metadata.getSerializedSize(), compressionType);
+ }
+
+ return managedCursorInfo.toByteArray();
Review Comment:
the compressed data is self-explain with `MLDataFormats.ManagedLedgerInfoMetadata` padding before the compressed data content. the parse logic will try to read magic number bytes. if magic number not exists, the data will be thought no compression is applied.
--
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] lifepuzzlefun commented on pull request #19561: [improve][meta] Support set metadata size threshold for compression.
Posted by "lifepuzzlefun (via GitHub)" <gi...@apache.org>.
lifepuzzlefun commented on PR #19561:
URL: https://github.com/apache/pulsar/pull/19561#issuecomment-1461538388
@poorbarcode @Technoboy- @315157973 can this pr be merged. if need change please let me know : - )
--
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] 315157973 commented on pull request #19561: [improve][meta] Support set metadata size threshold for compression.
Posted by "315157973 (via GitHub)" <gi...@apache.org>.
315157973 commented on PR #19561:
URL: https://github.com/apache/pulsar/pull/19561#issuecomment-1469599210
> @315157973 @poorbarcode @Technoboy- can this pr be merged？
The button is unavailable due to unit tests not all passing
--
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] congbobo184 commented on a diff in pull request #19561: [improve][meta] Support set metadata size threshold for compression.
Posted by "congbobo184 (via GitHub)" <gi...@apache.org>.
congbobo184 commented on code in PR #19561:
URL: https://github.com/apache/pulsar/pull/19561#discussion_r1137204324
##########
managed-ledger/src/main/java/org/apache/bookkeeper/mledger/ManagedLedgerFactoryConfig.java:
##########
@@ -92,8 +92,30 @@ public class ManagedLedgerFactoryConfig {
*/
private String managedLedgerInfoCompressionType = MLDataFormats.CompressionType.NONE.name();
+ /**
+ * ManagedLedgerInfo compression threshold. If the origin metadata size below configuration.
+ * compression will not apply.
+ */
+ private int managedLedgerInfoCompressionSizeThresholdBytes = 0;
Review Comment:
Where can we change this value? I haven't seen any opreation that we can change this value. Maybe we should create a PIP to add these two configurations?
--
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] 315157973 commented on a diff in pull request #19561: [improve][meta] Support set metadata size threshold for compression.
Posted by "315157973 (via GitHub)" <gi...@apache.org>.
315157973 commented on code in PR #19561:
URL: https://github.com/apache/pulsar/pull/19561#discussion_r1113018463
##########
managed-ledger/src/main/java/org/apache/bookkeeper/mledger/impl/MetaStoreImpl.java:
##########
@@ -415,29 +400,43 @@ private static MetaStoreException getException(Throwable t) {
}
public byte[] compressLedgerInfo(ManagedLedgerInfo managedLedgerInfo) {
- if (ledgerInfoCompressionType.equals(CompressionType.NONE)) {
+ CompressionType compressionType = ledgerInfoCompressionConfig.getCompressionType();
+ if (compressionType.equals(CompressionType.NONE)) {
return managedLedgerInfo.toByteArray();
}
- MLDataFormats.ManagedLedgerInfoMetadata mlInfoMetadata = MLDataFormats.ManagedLedgerInfoMetadata
- .newBuilder()
- .setCompressionType(ledgerInfoCompressionType)
- .setUncompressedSize(managedLedgerInfo.getSerializedSize())
- .build();
- return compressManagedInfo(managedLedgerInfo.toByteArray(), mlInfoMetadata.toByteArray(),
- mlInfoMetadata.getSerializedSize(), ledgerInfoCompressionType);
+
+ int uncompressedSize = managedLedgerInfo.getSerializedSize();
+ if (uncompressedSize > ledgerInfoCompressionConfig.getCompressThreshold()) {
+ MLDataFormats.ManagedLedgerInfoMetadata mlInfoMetadata = MLDataFormats.ManagedLedgerInfoMetadata
+ .newBuilder()
+ .setCompressionType(compressionType)
+ .setUncompressedSize(uncompressedSize)
+ .build();
+ return compressManagedInfo(managedLedgerInfo.toByteArray(), mlInfoMetadata.toByteArray(),
+ mlInfoMetadata.getSerializedSize(), compressionType);
+ }
+
+ return managedLedgerInfo.toByteArray();
}
public byte[] compressCursorInfo(ManagedCursorInfo managedCursorInfo) {
- if (cursorInfoCompressionType.equals(CompressionType.NONE)) {
+ CompressionType compressionType = cursorInfoCompressionConfig.getCompressionType();
+ if (compressionType.equals(CompressionType.NONE)) {
return managedCursorInfo.toByteArray();
}
- MLDataFormats.ManagedCursorInfoMetadata metadata = MLDataFormats.ManagedCursorInfoMetadata
- .newBuilder()
- .setCompressionType(cursorInfoCompressionType)
- .setUncompressedSize(managedCursorInfo.getSerializedSize())
- .build();
- return compressManagedInfo(managedCursorInfo.toByteArray(), metadata.toByteArray(),
- metadata.getSerializedSize(), cursorInfoCompressionType);
+
+ int uncompressedSize = managedCursorInfo.getSerializedSize();
+ if (uncompressedSize > cursorInfoCompressionConfig.getCompressThreshold()) {
+ MLDataFormats.ManagedCursorInfoMetadata metadata = MLDataFormats.ManagedCursorInfoMetadata
+ .newBuilder()
+ .setCompressionType(compressionType)
+ .setUncompressedSize(uncompressedSize)
+ .build();
+ return compressManagedInfo(managedCursorInfo.toByteArray(), metadata.toByteArray(),
+ metadata.getSerializedSize(), compressionType);
+ }
+
+ return managedCursorInfo.toByteArray();
Review Comment:
What if it is uncompressed data when decompressing?
--
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