[GitHub] [pulsar] AnonHxy commented on a diff in pull request #16824: [improve][doc] Improve defaultRetentionTimeInMinutes and defaultRetentionSizeInMB doc
AnonHxy commented on code in PR #16824: URL: https://github.com/apache/pulsar/pull/16824#discussion_r940905081 ## conf/broker.conf: ## @@ -1264,10 +1264,10 @@ replicatorPrefix=pulsar.repl # due to missing ZooKeeper watch (disable with value 0) replicationPolicyCheckDurationSeconds=600 -# Default message retention time +# Default message retention time. Using a value of -1, is disabling message retention time limit defaultRetentionTimeInMinutes=0 -# Default retention size +# Default retention size. Using a value of -1, is disabling message retention size limit Review Comment: Great. Have updated, PTAL ~ @codelipenghui @Anonymitaet -- 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-unsubscr...@pulsar.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [pulsar] michaeljmarshall commented on pull request #14488: [improve][broker] refactor ManagedLedger cacheEvictionTask implement
michaeljmarshall commented on PR #14488: URL: https://github.com/apache/pulsar/pull/14488#issuecomment-1208945845 Was this discussed on the mailing list? I did a quick search and couldn't find a thread on the topic. > We can do some optimizes for this logic. @aloyszhang - did you benchmark this change? I thought the previous implementation was intentionally designed to decrease GC overhead. -- 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-unsubscr...@pulsar.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [pulsar] AnonHxy commented on pull request #16824: [improve][doc] Improve defaultRetentionTimeInMinutes and defaultRetentionSizeInMB doc
AnonHxy commented on PR #16824: URL: https://github.com/apache/pulsar/pull/16824#issuecomment-1208945527 I find this PR [Default message retention time](https://github.com/apache/pulsar/pull/16572) has already improved partial of the document. So what I need do in this PR is complete the rest -- 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-unsubscr...@pulsar.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [pulsar] michaeljmarshall commented on pull request #16255: [client][python] getLastMessageIdAsync C binding
michaeljmarshall commented on PR #16255: URL: https://github.com/apache/pulsar/pull/16255#issuecomment-1208936875 > @BewareMyPower @michaeljmarshall Thanks for your explanation! I have a question about the broker and client of old version, if the broker supports the xx feature, but the client doesn't support it, shouldn't we cherry-pick it? One reason I think it is unnecessary to backport features to older versions of the client is that the protocol has a feature handshake built in that is supposed to enable different versions of the clients and servers to work together seamlessly. Therefore, if a user wants to get a new feature in the client that was already supported on the broker in an older version, they should be able to upgrade without issue. That being said, I am not sure that we document the client/server compatibility anywhere and I am not sure how good the test coverage is for the many permutations of client versions. -- 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-unsubscr...@pulsar.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [pulsar] AnonHxy commented on a diff in pull request #16824: [improve][doc] Improve defaultRetentionTimeInMinutes and defaultRetentionSizeInMB doc
AnonHxy commented on code in PR #16824: URL: https://github.com/apache/pulsar/pull/16824#discussion_r940905081 ## conf/broker.conf: ## @@ -1264,10 +1264,10 @@ replicatorPrefix=pulsar.repl # due to missing ZooKeeper watch (disable with value 0) replicationPolicyCheckDurationSeconds=600 -# Default message retention time +# Default message retention time. Using a value of -1, is disabling message retention time limit defaultRetentionTimeInMinutes=0 -# Default retention size +# Default retention size. Using a value of -1, is disabling message retention size limit Review Comment: Great. Have updated, PTAL ~ @codelipenghui @Anonymitaet -- 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-unsubscr...@pulsar.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [pulsar] tisonkun commented on issue #16802: Repeated messages of shared dispatcher
tisonkun commented on issue #16802: URL: https://github.com/apache/pulsar/issues/16802#issuecomment-1208934884 ``` /pulsar/pulsar-client-cpp/tests/BasicEndToEndTest.cc:2184: Failure Expected equality of these values: ResultTimeout Which is: TimeOut consumer.receive(m, 1000) Which is: Ok [ FAILED ] BasicEndToEndTest.testPatternMultiTopicsConsumerPubSub (1173 ms) [--] 1 test from BasicEndToEndTest (1173 ms total) [--] Global test environment tear-down [==] 1 test from 1 test suite ran. (1173 ms total) [ PASSED ] 0 tests. [ FAILED ] 1 test, listed below: [ FAILED ] BasicEndToEndTest.testPatternMultiTopicsConsumerPubSub ``` -- 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-unsubscr...@pulsar.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [pulsar] tisonkun commented on issue #16802: Repeated messages of shared dispatcher
tisonkun commented on issue #16802: URL: https://github.com/apache/pulsar/issues/16802#issuecomment-1208934327 https://github.com/apache/pulsar/runs/7739162912?check_suite_focus=true This can be another failure case of `BasicEndToEndTest.testPatternMultiTopicsConsumerPubSub`. -- 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-unsubscr...@pulsar.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [pulsar] github-actions[bot] commented on pull request #17006: [improve][admin] Not allow to terminate system topic.
github-actions[bot] commented on PR #17006: URL: https://github.com/apache/pulsar/pull/17006#issuecomment-1208932105 @Technoboy- Please provide a correct documentation label for your PR. Instructions see [Pulsar Documentation Label Guide](https://docs.google.com/document/d/1Qw7LHQdXWBW9t2-r-A7QdFDBwmZh6ytB4guwMoXHqc0). -- 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-unsubscr...@pulsar.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [pulsar] youzipi commented on issue #16911: [Doc] wrong pip in 2.10.0 release notes
youzipi commented on issue #16911: URL: https://github.com/apache/pulsar/issues/16911#issuecomment-1208927007 > And they were all listed in the 2.10.0 release note. You can see the PR for Negative acknowledgment backoff in the 'Clients' section. no, the nack PR is not listed there. I think the duplicate PIP number is the reason. https://pulsar.apache.org/release-notes/versioned/pulsar-2.10.0#clients  -- 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-unsubscr...@pulsar.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [pulsar] BewareMyPower commented on pull request #16255: [client][python] getLastMessageIdAsync C binding
BewareMyPower commented on PR #16255: URL: https://github.com/apache/pulsar/pull/16255#issuecomment-1208926338 > if the broker supports the xx feature, but the client doesn't support it IMO no as I've said: > The feature catch up should not be cherry-picked. -- 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-unsubscr...@pulsar.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [pulsar] Technoboy- opened a new pull request, #17006: [improve][admin] Not allow to terminate system topic.
Technoboy- opened a new pull request, #17006: URL: https://github.com/apache/pulsar/pull/17006 Fixes #15891 Master Issue: #15891 ### Motivation Pulsar-manager could see all the topics including the system topic and can terminate them. If terminates system topic by mistake, there would cause many serious problems. So it's better not allow to terminate the system topic. ### Documentation - [x] `doc-not-needed` (Please explain why) -- 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-unsubscr...@pulsar.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [pulsar] AnonHxy commented on a diff in pull request #16824: [improve][doc] Improve defaultRetentionTimeInMinutes and defaultRetentionSizeInMB doc
AnonHxy commented on code in PR #16824: URL: https://github.com/apache/pulsar/pull/16824#discussion_r940905081 ## conf/broker.conf: ## @@ -1264,10 +1264,10 @@ replicatorPrefix=pulsar.repl # due to missing ZooKeeper watch (disable with value 0) replicationPolicyCheckDurationSeconds=600 -# Default message retention time +# Default message retention time. Using a value of -1, is disabling message retention time limit defaultRetentionTimeInMinutes=0 -# Default retention size +# Default retention size. Using a value of -1, is disabling message retention size limit Review Comment: Great -- 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-unsubscr...@pulsar.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [pulsar] AnonHxy commented on a diff in pull request #16946: [Improve][Broker]Reduce GetReplicatedSubscriptionStatus local REST call
AnonHxy commented on code in PR #16946:
URL: https://github.com/apache/pulsar/pull/16946#discussion_r940903736
##
pulsar-broker/src/main/java/org/apache/pulsar/broker/admin/impl/PersistentTopicsBase.java:
##
@@ -5145,27 +5145,54 @@ protected void
internalGetReplicatedSubscriptionStatus(AsyncResponse asyncRespon
.thenCompose(__ ->
getPartitionedTopicMetadataAsync(topicName, authoritative, false))
.thenAccept(partitionMetadata -> {
if (partitionMetadata.partitions > 0) {
-final List>> futures =
-
Lists.newArrayListWithCapacity(partitionMetadata.partitions);
-final Map status =
Maps.newHashMap();
+List> futures = new
ArrayList<>(partitionMetadata.partitions);
+Map status = Maps.newHashMap();
for (int i = 0; i < partitionMetadata.partitions;
i++) {
TopicName partition =
topicName.getPartition(i);
-try {
-
futures.add(pulsar().getAdminClient().topics().getReplicatedSubscriptionStatusAsync(
-partition.toString(),
subName).whenComplete((response, throwable) -> {
-if (throwable != null) {
-log.error("[{}] Failed to get
replicated subscriptions on {} {}",
-clientAppId(), partition,
subName, throwable);
-asyncResponse.resume(new
RestException(throwable));
+futures.add(
+
pulsar().getNamespaceService().isServiceUnitOwnedAsync(partition)
+.thenCompose(owned -> {
+if (owned) {
+// if this broker owned the
partition do action like
+//
`internalGetReplicatedSubscriptionStatusForNonPartitionedTopic()`
+return
getTopicReferenceAsync(partition)
+.thenApply(topic -> {
Review Comment:
Yes. But `FutureUtil.waitForAll(futures)` on line5195 will handle this
exception @codelipenghui
--
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-unsubscr...@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
[GitHub] [pulsar] lhotari commented on a diff in pull request #15640: [Autorecovery] Default reppDnsResolverClass to ZkBookieRackAffinityMapping
lhotari commented on code in PR #15640: URL: https://github.com/apache/pulsar/pull/15640#discussion_r940899276 ## conf/bookkeeper.conf: ## @@ -277,6 +277,13 @@ useV2WireProtocol=true # # ensemblePlacementPolicy=org.apache.bookkeeper.client.RackawareEnsemblePlacementPolicy +# The DNS resolver class used for resolving network locations for bookies. The setting is used Review Comment: btw. I'm not sure if bookie client is used in bookkeeper for other than autorecovery. -- 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-unsubscr...@pulsar.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [pulsar] shibd commented on pull request #16969: [refactor][client c++] Delete PartitionedConsumerImpl, use MultiTopicsConsumerImpl instead
shibd commented on PR #16969: URL: https://github.com/apache/pulsar/pull/16969#issuecomment-1208916032 /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-unsubscr...@pulsar.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [pulsar] michaeljmarshall commented on a diff in pull request #15640: [Autorecovery] Default reppDnsResolverClass to ZkBookieRackAffinityMapping
michaeljmarshall commented on code in PR #15640: URL: https://github.com/apache/pulsar/pull/15640#discussion_r940893174 ## conf/bookkeeper.conf: ## @@ -277,6 +277,13 @@ useV2WireProtocol=true # # ensemblePlacementPolicy=org.apache.bookkeeper.client.RackawareEnsemblePlacementPolicy +# The DNS resolver class used for resolving network locations for bookies. The setting is used 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-unsubscr...@pulsar.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [pulsar] michaeljmarshall commented on a diff in pull request #15640: [Autorecovery] Default reppDnsResolverClass to ZkBookieRackAffinityMapping
michaeljmarshall commented on code in PR #15640: URL: https://github.com/apache/pulsar/pull/15640#discussion_r940885069 ## conf/bookkeeper.conf: ## @@ -277,6 +277,13 @@ useV2WireProtocol=true # # ensemblePlacementPolicy=org.apache.bookkeeper.client.RackawareEnsemblePlacementPolicy +# The DNS resolver class used for resolving network locations for bookies. The setting is used Review Comment: Good point. I'll resolve conflicts and update the comments. -- 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-unsubscr...@pulsar.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [pulsar] tisonkun opened a new pull request, #17005: [improve][test] Avoid hacky modify static final field
tisonkun opened a new pull request, #17005: URL: https://github.com/apache/pulsar/pull/17005 Master Issue: #16912 - [x] `doc-not-needed` cc @Shoothzj @eolivelli -- 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-unsubscr...@pulsar.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [pulsar] lhotari commented on a diff in pull request #15640: [Autorecovery] Default reppDnsResolverClass to ZkBookieRackAffinityMapping
lhotari commented on code in PR #15640: URL: https://github.com/apache/pulsar/pull/15640#discussion_r940880414 ## conf/bookkeeper.conf: ## @@ -277,6 +277,13 @@ useV2WireProtocol=true # # ensemblePlacementPolicy=org.apache.bookkeeper.client.RackawareEnsemblePlacementPolicy +# The DNS resolver class used for resolving network locations for bookies. The setting is used Review Comment: I wonder if it should be mentioned that this setting would be used for the bookie client used in bookies for autorecovery and compaction? It would also be worth mentioning that this setting should match the setting for bookie clients in broker. In broker.conf, ZkBookieRackAffinityMapping will be used when `bookkeeperClientRackawarePolicyEnabled=true`. This is mentioned in this comment: https://github.com/apache/pulsar/issues/151#issuecomment-268802489 -- 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-unsubscr...@pulsar.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [pulsar] shibd commented on pull request #16969: [refactor][client c++] Delete PartitionedConsumerImpl, use MultiTopicsConsumerImpl instead
shibd commented on PR #16969: URL: https://github.com/apache/pulsar/pull/16969#issuecomment-1208892657 /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-unsubscr...@pulsar.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [pulsar] lhotari commented on pull request #15640: [Autorecovery] Default reppDnsResolverClass to ZkBookieRackAffinityMapping
lhotari commented on PR #15640: URL: https://github.com/apache/pulsar/pull/15640#issuecomment-1208891693 Related explanation about ZkBookieRackAffinityMapping: https://github.com/apache/pulsar/issues/151#issuecomment-268802489 . On the broker side, ZkBookieRackAffinityMapping will get used for the bookie client when `bookkeeperClientRackawarePolicyEnabled=true` in `broker.conf`. -- 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-unsubscr...@pulsar.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [pulsar] zymap commented on issue #14861: Broker OOM when the bookie is slowly to respond and AQ is smaller than WQ
zymap commented on issue #14861: URL: https://github.com/apache/pulsar/issues/14861#issuecomment-1208891020 If you configured the write quorum as 3, I'd suggest you increase the bookie node to 4 or more. After configuring the back pressure, it should be normal to have that exception. Because once a bookie is slow, the bookie will not available for writing and it needs to find another bookie to write. If you only have 3 bookies. I'd suggest you to configure the WQ and AQ as 2 to avoid this. -- 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-unsubscr...@pulsar.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [pulsar] lhotari commented on pull request #15640: [Autorecovery] Default reppDnsResolverClass to ZkBookieRackAffinityMapping
lhotari commented on PR #15640: URL: https://github.com/apache/pulsar/pull/15640#issuecomment-1208890216 I think this is a bug and doesn't need a PIP. -- 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-unsubscr...@pulsar.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [pulsar] momo-jun commented on a diff in pull request #17004: [fix][doc] Fix wrong description of `pulsar.allocator.leak_detection`
momo-jun commented on code in PR #17004: URL: https://github.com/apache/pulsar/pull/17004#discussion_r940873542 ## site2/docs/client-libraries-java.md: ## @@ -168,7 +168,7 @@ You can set the client memory allocator configurations through Java properties.< |---|---|---|---|--- `pulsar.allocator.pooled` | String | If set to `true`, the client uses a direct memory pool. If set to `false`, the client uses a heap memory without pool | true | true false `pulsar.allocator.exit_on_oom` | String | Whether to exit the JVM when OOM happens | false | true false -`pulsar.allocator.leak_detection` | String | Service URL provider for Pulsar service | Disabled | Disabled Simple Advanced Paranoid +`pulsar.allocator.leak_detection` | String | The leak detection policy for Pulsar bytebuf allocator. **Disabled**: No leak detection and no overhead. **Simple**: Instruments 1% of the allocated buffer to track for leaks. **Advanced**: Instruments 1% of the allocated buffer to track for leaks, reporting stack traces of places where the buffer was used. **Paranoid**: Instruments 100% of the allocated buffer to track for leaks, reporting stack traces of places where the buffer was used. Introduce very significant overhead. | Disabled | Disabled Simple Advanced Paranoid Review Comment: ```suggestion `pulsar.allocator.leak_detection` | String | The leak detection policy for Pulsar bytebuf allocator. **Disabled**: No leak detection and no overhead. **Simple**: Instruments 1% of the allocated buffer to track for leaks. **Advanced**: Instruments 1% of the allocated buffer to track for leaks, reporting stack traces of places where the buffer is used. **Paranoid**: Instruments 100% of the allocated buffer to track for leaks, reporting stack traces of places where the buffer is used and introduces a significant overhead. | Disabled | Disabled Simple Advanced Paranoid ``` -- 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-unsubscr...@pulsar.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [pulsar] nodece commented on pull request #16255: [client][python] getLastMessageIdAsync C binding
nodece commented on PR #16255: URL: https://github.com/apache/pulsar/pull/16255#issuecomment-1208884720 @BewareMyPower @michaeljmarshall Thanks for your explanation! I have a question about the broker and client of old version, if the broker supports the xx feature, but the client doesn't support it, shouldn't we cherry-pick it? -- 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-unsubscr...@pulsar.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [pulsar] michaeljmarshall commented on pull request #16825: Fix rack awareness cache expiration data race
michaeljmarshall commented on PR #16825: URL: https://github.com/apache/pulsar/pull/16825#issuecomment-1208875618 @BewareMyPower - yes, I will take care of cherry picking this PR. -- 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-unsubscr...@pulsar.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [pulsar] michaeljmarshall commented on pull request #16255: [client][python] getLastMessageIdAsync C binding
michaeljmarshall commented on PR #16255: URL: https://github.com/apache/pulsar/pull/16255#issuecomment-1208874947 > My personal answer is no. The feature catch up should not be cherry-picked. For the same reason, if users want some new features of a specific component, they have to upgrade the major version. I agree with this line of thinking. My only reason for asking in this case is because branch-2.11 was only just recently created and the RC has not yet been tagged. I am not going to ask on the ML because I'll just let this target 2.12. -- 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-unsubscr...@pulsar.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [pulsar] codelipenghui commented on a diff in pull request #12258: [pulsar-broker] Support caching to drain backlog consumers
codelipenghui commented on code in PR #12258:
URL: https://github.com/apache/pulsar/pull/12258#discussion_r940860505
##
managed-ledger/src/main/java/org/apache/bookkeeper/mledger/impl/OpAddEntry.java:
##
@@ -213,8 +213,11 @@ public void safeRun() {
EntryImpl entry = EntryImpl.create(ledger.getId(), entryId, data);
// EntryCache.insert: duplicates entry by allocating new entry and
data. so, recycle entry after calling
// insert
-ml.entryCache.insert(entry);
-entry.release();
+// Entry cache doesn't copy the data if entry already exist into
the cache.
+// Backlog read tries to add entry into cache which can try to add
duplicate entry into cache.
+if (ml.entryCache.insert(entry)) {
+entry.release();
+}
Review Comment:
Here will be a potential memory leak? If the cache doesn't have enough
space, will the entry not be released?
--
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-unsubscr...@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
[GitHub] [pulsar] RobertIndie commented on issue #16911: [Doc] wrong pip in 2.10.0 release notes
RobertIndie commented on issue #16911: URL: https://github.com/apache/pulsar/issues/16911#issuecomment-1208871849 They were both released in the 2.10.0: * [PIP 106] Broker extensions to provide operators of enterprise-wide clusters better control and flexibility : https://github.com/apache/pulsar/pull/12536 * PIP 106: Negative acknowledgment backoff : https://github.com/apache/pulsar/pull/12566 And they were all described in the 2.10.0 release note. You can see the PR for Negative acknowledgment backoff in the 'Clients' section. -- 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-unsubscr...@pulsar.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [pulsar] BewareMyPower commented on pull request #16255: [client][python] getLastMessageIdAsync C binding
BewareMyPower commented on PR #16255: URL: https://github.com/apache/pulsar/pull/16255#issuecomment-1208870161 @michaeljmarshall I think it needs a discussion. There was a similar discussion here: https://github.com/apache/pulsar/pull/15822#issuecomment-1206270637 /cc @nodece The core question is that should we cherry-pick all these new features of C++/Python clients to release branches? My personal answer is no. The feature catch up should not be cherry-picked. For the same reason, if users want some new features of a specific component, they have to upgrade the major version. Regarding 2.11.0 release, I think you can reply in the mail list. I see branch-2.11 has already been created. -- 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-unsubscr...@pulsar.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [pulsar] michaeljmarshall commented on pull request #16255: [client][python] getLastMessageIdAsync C binding
michaeljmarshall commented on PR #16255: URL: https://github.com/apache/pulsar/pull/16255#issuecomment-1208865641 @komalatammal @eolivelli @BewareMyPower - it'd be nice to cherry pick this to 2.11 since it is a feature that is already in the Java client api. Do you think it is too late to get this change into that branch? -- 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-unsubscr...@pulsar.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[pulsar] branch master updated: [client][python] getLastMessageIdAsync C binding (#16255)
This is an automated email from the ASF dual-hosted git repository.
mmarshall pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/pulsar.git
The following commit(s) were added to refs/heads/master by this push:
new fd8ebaa522b [client][python] getLastMessageIdAsync C binding (#16255)
fd8ebaa522b is described below
commit fd8ebaa522bb8e7525be249e34913375c9c6f236
Author: komalatammal <107959336+komalatam...@users.noreply.github.com>
AuthorDate: Mon Aug 8 23:19:42 2022 -0400
[client][python] getLastMessageIdAsync C binding (#16255)
* python cc binding for getLastMessageId
* add python Consumer class method and doc
* fix linter issues based on clang-format
* ubuntu linter fix
* try run unit test in ci
* fix doc comment
* test the test case can be ran
### Motivation
Python function getLastMessageId
It is a C binding for https://github.com/apache/pulsar/pull/16182 to
implement get_last_message_id() in Python client.
### Modifications
Add Python/C binding code for get_last_message_id()
### Verifying this change
It compiles.
- [x] Make sure that the change passes the CI checks.
This change is a trivial rework / code cleanup without any test coverage.
### Does this pull request potentially affect one of the following parts:
*If `yes` was chosen, please highlight the changes*
- Dependencies (does it add or upgrade a dependency): (no)
- The public API: (yes)
- The schema: (no)
- The default values of configurations: (no)
- The wire protocol: (no)
- The rest endpoints: (no)
- The admin cli options: (no)
- Anything that affects deployment: (no)
### Documentation
Check the box below or label this PR directly.
Need to update docs?
- [ ] `doc-required`
(Your PR needs to update docs and you will update later)
- [ ] `doc-not-needed`
- [x] `doc`
Python Doc is updated in __init__.py
- [ ] `doc-complete`
(Docs have been already added)
---
pulsar-client-cpp/README.md | 2 +-
pulsar-client-cpp/include/pulsar/c/consumer.h | 3 +++
pulsar-client-cpp/lib/c/c_Consumer.cc | 5 +
pulsar-client-cpp/python/pulsar/__init__.py | 7 ++-
pulsar-client-cpp/python/pulsar_test.py | 12
pulsar-client-cpp/python/src/consumer.cc | 13 -
6 files changed, 39 insertions(+), 3 deletions(-)
diff --git a/pulsar-client-cpp/README.md b/pulsar-client-cpp/README.md
index 3dfa169c923..155e6c6a907 100644
--- a/pulsar-client-cpp/README.md
+++ b/pulsar-client-cpp/README.md
@@ -281,7 +281,7 @@ ${PULSAR_PATH}/pulsar-test-service-stop.sh
## Requirements for Contributors
-It's recommended to install [LLVM](https://llvm.org/builds/) for `clang-tidy`
and `clang-format`. Pulsar C++ client use `clang-format` 6.0+ to format files.
+It's required to install [LLVM](https://llvm.org/builds/) for `clang-tidy` and
`clang-format`. Pulsar C++ client use `clang-format` 6.0+ to format files.
`make format` automatically formats the files.
Use `pulsar-client-cpp/docker-format.sh` to ensure the C++ sources are
correctly formatted.
diff --git a/pulsar-client-cpp/include/pulsar/c/consumer.h
b/pulsar-client-cpp/include/pulsar/c/consumer.h
index 03f80f32394..37fd2acf5b8 100644
--- a/pulsar-client-cpp/include/pulsar/c/consumer.h
+++ b/pulsar-client-cpp/include/pulsar/c/consumer.h
@@ -236,6 +236,9 @@ PULSAR_PUBLIC pulsar_result
pulsar_consumer_seek(pulsar_consumer_t *consumer, pu
PULSAR_PUBLIC int pulsar_consumer_is_connected(pulsar_consumer_t *consumer);
+PULSAR_PUBLIC pulsar_result
pulsar_consumer_get_last_message_id(pulsar_consumer_t *consumer,
+
pulsar_message_id_t *messageId);
+
#ifdef __cplusplus
}
#endif
diff --git a/pulsar-client-cpp/lib/c/c_Consumer.cc
b/pulsar-client-cpp/lib/c/c_Consumer.cc
index 9917e8cfad6..00d8311f132 100644
--- a/pulsar-client-cpp/lib/c/c_Consumer.cc
+++ b/pulsar-client-cpp/lib/c/c_Consumer.cc
@@ -143,3 +143,8 @@ pulsar_result pulsar_consumer_seek(pulsar_consumer_t
*consumer, pulsar_message_i
}
int pulsar_consumer_is_connected(pulsar_consumer_t *consumer) { return
consumer->consumer.isConnected(); }
+
+pulsar_result pulsar_consumer_get_last_message_id(pulsar_consumer_t *consumer,
+ pulsar_message_id_t
*messageId) {
+return
(pulsar_result)consumer->consumer.getLastMessageId(messageId->messageId);
+}
diff --git a/pulsar-client-cpp/python/pulsar/__init__.py
b/pulsar-client-cpp/python/pulsar/__init__.py
index e79955b57dd..3832c3e69e2 100644
--- a/pulsar-client-cpp/python/pulsar/__init__.py
+++ b/pulsar-client-cpp/python/pulsar/__init__.py
@@ -1253,7 +1253,12 @@ class Consumer:
Check if
[GitHub] [pulsar] michaeljmarshall merged pull request #16255: [client][python] getLastMessageIdAsync C binding
michaeljmarshall merged PR #16255: URL: https://github.com/apache/pulsar/pull/16255 -- 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-unsubscr...@pulsar.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [pulsar] RobertIndie opened a new pull request, #17004: [fix][doc] Fix wrong description of `pulsar.allocator.leak_detection`
RobertIndie opened a new pull request, #17004: URL: https://github.com/apache/pulsar/pull/17004 Fixes #16804 ### Motivation The property "pulsar.allocator.leak_detection" has wrong description: "Service URL provider for Pulsar service" in https://pulsar.apache.org/docs/client-libraries-java/#client-memory-allocator-configuration ### Modifications * Fix the wrong description. ### Documentation Check the box below or label this PR directly. Need to update docs? - [ ] `doc-required` (Your PR needs to update docs and you will update later) - [ ] `doc-not-needed` (Please explain why) - [x] `doc` (Your PR contains doc changes) - [ ] `doc-complete` (Docs have been already added) -- 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-unsubscr...@pulsar.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [pulsar] liliang950210 commented on issue #16977: Use key story tls connect to broker,but IP change to 127.0.0.1 and connect failed
liliang950210 commented on issue #16977: URL: https://github.com/apache/pulsar/issues/16977#issuecomment-1208863623 I found this problem. I set advertisedAddress=127.0.0.1 to 127.0.0.1 in the broker, but bindAddress is not configured. As a result, the broker listens on the entire network and external systems can access the service, but an error is reported during the connection. Change the value of advertisedAddress to the corresponding IP address. -- 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-unsubscr...@pulsar.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [pulsar] michaeljmarshall commented on pull request #16182: [client][c++] add getLastMessageIdAsync in Consumer
michaeljmarshall commented on PR #16182: URL: https://github.com/apache/pulsar/pull/16182#issuecomment-1208862724 > yes, it is a strict constraint for all non-trivial changes. @eolivelli - I did not realize this was a strict requirement. Have we discussed this on the dev mailing list recently? It might be worth re-iterating the requirement. It might also be worth documenting here: https://github.com/apache/pulsar/wiki/Committer-Guide. -- 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-unsubscr...@pulsar.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [pulsar] tisonkun commented on pull request #16902: [improve][repo] Tidy issue templates
tisonkun commented on PR #16902: URL: https://github.com/apache/pulsar/pull/16902#issuecomment-1208856013 @Anonymitaet thank you! Although, this patch changes issue templates only. As long as there's no license issue or format failure, code related failure should not be a blocker. -- 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-unsubscr...@pulsar.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[pulsar] branch master updated: [improve][broker]Remove unnecessary lock on the stats thread (#16983)
This is an automated email from the ASF dual-hosted git repository.
yong pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/pulsar.git
The following commit(s) were added to refs/heads/master by this push:
new 4d5ecba9394 [improve][broker]Remove unnecessary lock on the stats
thread (#16983)
4d5ecba9394 is described below
commit 4d5ecba9394515e7dbf19fd01739c1e1dc90e5ec
Author: Yong Zhang
AuthorDate: Tue Aug 9 10:57:20 2022 +0800
[improve][broker]Remove unnecessary lock on the stats thread (#16983)
---
*Motivation*
We found there has a block between the pulsar-ordered executor and
the pulsar-stats-updater executor.
The pulsar-ordered executor is trying to createManagedLedgerOffloader,
and the pulsar-stats-updater is getting the compactor. Both them want
to get the lock.
We have an improvement about the `createManagedLedgerOffloader` before.
https://github.com/apache/pulsar/pull/15883
We are using `getCompactor(false)` for the stats related operations.
The `getCompactor` is guarded by `synchronized`. Actually, the stats
just want to get the current compactor without initializing it. We
don't need to use `synchronized` to guard this operation.
*Modification*
Remove unnecessary `synchronized` on the `getCompactor` method.
---
.../main/java/org/apache/pulsar/broker/PulsarService.java | 13 -
.../org/apache/pulsar/broker/service/BrokerService.java | 9 +++--
.../pulsar/broker/service/persistent/PersistentTopic.java | 7 +--
.../broker/stats/prometheus/NamespaceStatsAggregator.java | 8 +---
.../apache/pulsar/broker/stats/PrometheusMetricsTest.java | 2 +-
5 files changed, 14 insertions(+), 25 deletions(-)
diff --git
a/pulsar-broker/src/main/java/org/apache/pulsar/broker/PulsarService.java
b/pulsar-broker/src/main/java/org/apache/pulsar/broker/PulsarService.java
index b5baa717093..6e9a9442a6c 100644
--- a/pulsar-broker/src/main/java/org/apache/pulsar/broker/PulsarService.java
+++ b/pulsar-broker/src/main/java/org/apache/pulsar/broker/PulsarService.java
@@ -1419,16 +1419,19 @@ public class PulsarService implements AutoCloseable,
ShutdownService {
}
public synchronized Compactor getCompactor() throws PulsarServerException {
-return getCompactor(true);
-}
-
-public synchronized Compactor getCompactor(boolean shouldInitialize)
throws PulsarServerException {
-if (this.compactor == null && shouldInitialize) {
+if (this.compactor == null) {
this.compactor = newCompactor();
}
return this.compactor;
}
+// This method is used for metrics, which is allowed to as null
+// Because it's no operation on the compactor, so let's remove the
synchronized on this method
+// to avoid unnecessary lock competition.
+public Compactor getNullableCompactor() {
+return this.compactor;
+}
+
protected synchronized OrderedScheduler
getOffloaderScheduler(OffloadPoliciesImpl offloadPolicies) {
if (this.offloaderScheduler == null) {
this.offloaderScheduler = OrderedScheduler.newSchedulerBuilder()
diff --git
a/pulsar-broker/src/main/java/org/apache/pulsar/broker/service/BrokerService.java
b/pulsar-broker/src/main/java/org/apache/pulsar/broker/service/BrokerService.java
index 0a2e99004a8..e99fce6f68a 100644
---
a/pulsar-broker/src/main/java/org/apache/pulsar/broker/service/BrokerService.java
+++
b/pulsar-broker/src/main/java/org/apache/pulsar/broker/service/BrokerService.java
@@ -1990,12 +1990,9 @@ public class BrokerService implements Closeable {
}
topics.remove(topic);
-try {
-Compactor compactor = pulsar.getCompactor(false);
-if (compactor != null) {
-compactor.getStats().removeTopic(topic);
-}
-} catch (PulsarServerException ignore) {
+Compactor compactor = pulsar.getNullableCompactor();
+if (compactor != null) {
+compactor.getStats().removeTopic(topic);
}
}
diff --git
a/pulsar-broker/src/main/java/org/apache/pulsar/broker/service/persistent/PersistentTopic.java
b/pulsar-broker/src/main/java/org/apache/pulsar/broker/service/persistent/PersistentTopic.java
index 00e4bc01ec4..6673bedce23 100644
---
a/pulsar-broker/src/main/java/org/apache/pulsar/broker/service/persistent/PersistentTopic.java
+++
b/pulsar-broker/src/main/java/org/apache/pulsar/broker/service/persistent/PersistentTopic.java
@@ -1945,12 +1945,7 @@ public class PersistentTopic extends AbstractTopic
implements Topic, AddEntryCal
}
private Optional getCompactorMXBean() {
-Compactor compactor = null;
-try {
-compactor = brokerService.pulsar().getCompactor(false);
-} catch (PulsarServerException ex) {
-log.warn("get compactor error", ex);
-}
+
[GitHub] [pulsar] momo-jun commented on pull request #16939: [feature][function] add the ability to customize logging level for Go & Python functions
momo-jun commented on PR #16939: URL: https://github.com/apache/pulsar/pull/16939#issuecomment-1208855361 @tpiperatgod It will help users a lot if you can add the docs for this feature. Do you have any planned updates on that? -- 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-unsubscr...@pulsar.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [pulsar] zymap merged pull request #16983: [improve][broker]Remove unnecessary lock on the stats thread
zymap merged PR #16983: URL: https://github.com/apache/pulsar/pull/16983 -- 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-unsubscr...@pulsar.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[pulsar] branch master updated: [doc][workflow] Add Reporting Vulnerabilities section to Security Policy (#16962)
This is an automated email from the ASF dual-hosted git repository. liuyu pushed a commit to branch master in repository https://gitbox.apache.org/repos/asf/pulsar.git The following commit(s) were added to refs/heads/master by this push: new b1ad198e101 [doc][workflow] Add Reporting Vulnerabilities section to Security Policy (#16962) b1ad198e101 is described below commit b1ad198e101a106cac1c99f0bf812a1983c4fc2c Author: tison AuthorDate: Tue Aug 9 10:52:23 2022 +0800 [doc][workflow] Add Reporting Vulnerabilities section to Security Policy (#16962) --- SECURITY.md | 2 +- site2/docs/security-policy-and-supported-versions.md | 6 ++ 2 files changed, 7 insertions(+), 1 deletion(-) diff --git a/SECURITY.md b/SECURITY.md index c474eb7f4bd..7bd3ead079f 100644 --- a/SECURITY.md +++ b/SECURITY.md @@ -1,3 +1,3 @@ # Security Policy -The security policy and supported versions are outlined on the Pulsar website here: https://pulsar.apache.org/docs/security-policy-and-supported-versions/. \ No newline at end of file +The security policy and supported versions are outlined on the Pulsar website here: https://pulsar.apache.org/docs/security-policy-and-supported-versions/. diff --git a/site2/docs/security-policy-and-supported-versions.md b/site2/docs/security-policy-and-supported-versions.md index 23368650777..ac907e12c70 100644 --- a/site2/docs/security-policy-and-supported-versions.md +++ b/site2/docs/security-policy-and-supported-versions.md @@ -14,6 +14,12 @@ https://pulsar.apache.org/docs/en/security-overview/. The Pulsar community will announce security vulnerabilities and how to mitigate them on the [us...@pulsar.apache.org](mailto:us...@pulsar.apache.org). For instructions on how to subscribe, please see https://pulsar.apache.org/contact/. +## Reporting Vulnerabilities + +The Pulsar community follows the ASF [vulnerability handling process](https://apache.org/security/#vulnerability-handling). + +To report a new vulnerability you have discovered please follow the [ASF vulnerability reporting process](https://apache.org/security/#reporting-a-vulnerability). + ## Versioning Policy The Pulsar project adheres to [Semantic Versioning](http://semver.org/spec/v2.0.0.html). Existing releases can expect
[GitHub] [pulsar] Anonymitaet merged pull request #16962: Add Reporting Vulnerabilities section to Security Policy
Anonymitaet merged PR #16962: URL: https://github.com/apache/pulsar/pull/16962 -- 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-unsubscr...@pulsar.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [pulsar] Anonymitaet closed issue #16919: [Security] [Doc] no advice on how to report vulnerabilities
Anonymitaet closed issue #16919: [Security] [Doc] no advice on how to report vulnerabilities URL: https://github.com/apache/pulsar/issues/16919 -- 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-unsubscr...@pulsar.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[pulsar] branch branch-2.9 updated: [Branch 2.9][fix][broker] Upgrade log4j2 version to 2.18.0 (#16884) (#16995)
This is an automated email from the ASF dual-hosted git repository.
dezhiliu pushed a commit to branch branch-2.9
in repository https://gitbox.apache.org/repos/asf/pulsar.git
The following commit(s) were added to refs/heads/branch-2.9 by this push:
new 05d35d8d81f [Branch 2.9][fix][broker] Upgrade log4j2 version to 2.18.0
(#16884) (#16995)
05d35d8d81f is described below
commit 05d35d8d81ffb962ee303483865c97ba0652ddb3
Author: Dezhi LIiu <33149602+liudezhi2...@users.noreply.github.com>
AuthorDate: Tue Aug 9 10:50:28 2022 +0800
[Branch 2.9][fix][broker] Upgrade log4j2 version to 2.18.0 (#16884) (#16995)
* Upgrade log4j2 version to 2.18.0
Co-authored-by: liudezhi
---
buildtools/pom.xml | 2 +-
distribution/server/src/assemble/LICENSE.bin.txt | 8 ++--
pom.xml| 10 -
.../EnvironmentBasedSecretsProviderTest.java | 41 ++-
.../flume/node/TestEnvVarResolverProperties.java | 47 --
5 files changed, 43 insertions(+), 65 deletions(-)
diff --git a/buildtools/pom.xml b/buildtools/pom.xml
index e35e7bab1da..7bb238b7167 100644
--- a/buildtools/pom.xml
+++ b/buildtools/pom.xml
@@ -39,7 +39,7 @@
1.8
1.8
3.0.0-M3
-2.17.1
+2.18.0
1.7.32
7.3.0
3.11
diff --git a/distribution/server/src/assemble/LICENSE.bin.txt
b/distribution/server/src/assemble/LICENSE.bin.txt
index 6a228e7ea3f..669fe881b6f 100644
--- a/distribution/server/src/assemble/LICENSE.bin.txt
+++ b/distribution/server/src/assemble/LICENSE.bin.txt
@@ -390,10 +390,10 @@ The Apache Software License, Version 2.0
- jakarta.validation-jakarta.validation-api-2.0.2.jar
- javax.validation-validation-api-1.1.0.Final.jar
* Log4J
-- org.apache.logging.log4j-log4j-api-2.17.1.jar
-- org.apache.logging.log4j-log4j-core-2.17.1.jar
-- org.apache.logging.log4j-log4j-slf4j-impl-2.17.1.jar
-- org.apache.logging.log4j-log4j-web-2.17.1.jar
+- org.apache.logging.log4j-log4j-api-2.18.0.jar
+- org.apache.logging.log4j-log4j-core-2.18.0.jar
+- org.apache.logging.log4j-log4j-slf4j-impl-2.18.0.jar
+- org.apache.logging.log4j-log4j-web-2.18.0.jar
* Java Native Access JNA -- net.java.dev.jna-jna-4.2.0.jar
* BookKeeper
- org.apache.bookkeeper-bookkeeper-common-4.14.5.jar
diff --git a/pom.xml b/pom.xml
index 14aa41c09a3..64be13ca11d 100644
--- a/pom.xml
+++ b/pom.xml
@@ -120,7 +120,7 @@ flexible messaging model and an intuitive client
API.
6.10.2
1.7.32
3.2.2
-2.17.1
+2.18.0
1.69
1.0.2
2.13.2
@@ -137,6 +137,7 @@ flexible messaging model and an intuitive client
API.
0.19.0
${grpc.version}
2.8.9
+1.2.1
0.8.3
2.2.0
3.6.0
@@ -1279,6 +1280,13 @@ flexible messaging model and an intuitive client
API.
test
+
+ com.github.stefanbirkner
+ system-lambda
+ ${system-lambda.version}
+ test
+
+
org.powermock
powermock-module-testng
diff --git
a/pulsar-functions/secrets/src/test/java/org/apache/pulsar/functions/secretsprovider/EnvironmentBasedSecretsProviderTest.java
b/pulsar-functions/secrets/src/test/java/org/apache/pulsar/functions/secretsprovider/EnvironmentBasedSecretsProviderTest.java
index 1fc76b337a5..22ef2dd9e60 100644
---
a/pulsar-functions/secrets/src/test/java/org/apache/pulsar/functions/secretsprovider/EnvironmentBasedSecretsProviderTest.java
+++
b/pulsar-functions/secrets/src/test/java/org/apache/pulsar/functions/secretsprovider/EnvironmentBasedSecretsProviderTest.java
@@ -21,10 +21,7 @@ package org.apache.pulsar.functions.secretsprovider;
import static org.testng.Assert.assertEquals;
import static org.testng.Assert.assertNull;
-
-import java.lang.reflect.Field;
-import java.util.Map;
-
+import com.github.stefanbirkner.systemlambda.SystemLambda;
import org.testng.annotations.Test;
public class EnvironmentBasedSecretsProviderTest {
@@ -32,38 +29,8 @@ public class EnvironmentBasedSecretsProviderTest {
public void testConfigValidation() throws Exception {
EnvironmentBasedSecretsProvider provider = new
EnvironmentBasedSecretsProvider();
assertNull(provider.provideSecret("mySecretName", "Ignored"));
-injectEnvironmentVariable("mySecretName", "SecretValue");
-assertEquals(provider.provideSecret("mySecretName", "Ignored"),
"SecretValue");
-}
-
-private static void injectEnvironmentVariable(String key, String value)
-throws Exception {
-
-Class processEnvironment =
Class.forName("java.lang.ProcessEnvironment");
-
-Field unmodifiableMapField = getAccessibleField(processEnvironment,
"theUnmodifiableEnvironment");
-Object unmodifiableMap = unmodifiableMapField.get(null);
-injectIntoUnmodifiableMap(key, value, unmodifiableMap);
-
-Field mapField = getAccessibleField(processEnvironment,
"theEnvironment");
-
[GitHub] [pulsar] liudezhi2098 merged pull request #16995: [Branch 2.9][fix][broker] Upgrade log4j2 version to 2.18.0 (#16884)
liudezhi2098 merged PR #16995: URL: https://github.com/apache/pulsar/pull/16995 -- 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-unsubscr...@pulsar.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [pulsar] Anonymitaet commented on pull request #16902: [improve][repo] Tidy issue templates
Anonymitaet commented on PR #16902: URL: https://github.com/apache/pulsar/pull/16902#issuecomment-1208850346 One check was failed. I've triggered it. -- 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-unsubscr...@pulsar.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[pulsar] branch master updated (21634edd30b -> c29503e7c87)
This is an automated email from the ASF dual-hosted git repository. xiangying pushed a change to branch master in repository https://gitbox.apache.org/repos/asf/pulsar.git from 21634edd30b [improve][doc] Improve TLS transport encryption and authentication (#16924) add c29503e7c87 [Fix][Flaky-test] Fix testConsumeTxnMessage (#16981) No new revisions were added by this update. Summary of changes: .../apache/pulsar/testclient/PerformanceTransactionTest.java | 12 ++-- 1 file changed, 10 insertions(+), 2 deletions(-)
[GitHub] [pulsar] liangyepianzhou closed issue #14109: Flaky-test: PerformanceTransactionTest.testConsumeTxnMessage
liangyepianzhou closed issue #14109: Flaky-test: PerformanceTransactionTest.testConsumeTxnMessage URL: https://github.com/apache/pulsar/issues/14109 -- 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-unsubscr...@pulsar.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [pulsar] liangyepianzhou merged pull request #16981: [Fix][Flaky-test] Fix testConsumeTxnMessage
liangyepianzhou merged PR #16981: URL: https://github.com/apache/pulsar/pull/16981 -- 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-unsubscr...@pulsar.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [pulsar] congbobo184 opened a new pull request, #17003: [fix][broker] fix broker unackmessages number reduce error
congbobo184 opened a new pull request, #17003:
URL: https://github.com/apache/pulsar/pull/17003
issue: reduce the consumer `unackedCount` use incorrect consumer.
reproduce step
```
for (int i = 0; i < 5; i++) {
producer.newMessage().value(("Hello Pulsar - " +
i).getBytes()).sendAsync();
}
// consume-1 receive 5 batch messages
List list = new ArrayList<>();
for (int i = 0; i < 5; i++) {
list.add(consumer1.receive().getMessageId());
}
// consumer-1 redeliver the batch messages
consumer1.negativeAcknowledge(list.get(0));
// consumer-2 will receive the messages that the consumer-1
redelivered
for (int i = 0; i < 5; i++) {
consumer2.receive().getMessageId();
}
// consumer1 ack two messages in the batch message
consumer1.acknowledge(list.get(1));
consumer1.acknowledge(list.get(2));
// consumer-2 redeliver the rest of the messages
consumer2.negativeAcknowledge(list.get(1));
// consume-1 close will redeliver the rest messages to consumer-2
consumer1.close();
// consumer-2 can receive the rest of 3 messages
for (int i = 0; i < 3; i++) {
consumer2.acknowledge(consumer2.receive().getMessageId());
}
// consumer-2 can't receive any messages, all the messages in batch
has been acked
Message message = consumer2.receive(1, TimeUnit.SECONDS);
assertNull(message);
// the number of consumer-2's unacked messages is 0
Awaitility.await().until(() ->
getPulsar().getBrokerService().getTopic(topicName, false)
.get().get().getSubscription(subName).getConsumers().get(0).getUnackedMessages()
== 0);
```
in current code the `getUnackedMessages = 2`
### Motivation
get the correct consumer and reduce the correct un acked messages
### Modifications
change the method `getAckedCountForBatchIndexLevelEnabled` use
ownerConsumer to check the pendingAck messages
### Verifying this change
add the test
### Does this pull request potentially affect one of the following parts:
*If `yes` was chosen, please highlight the changes*
- Dependencies (does it add or upgrade a dependency): (no)
- The public API: (no)
- The schema: (no)
- The default values of configurations: (no)
- The wire protocol: (no)
- The rest endpoints: (no)
- The admin cli options: (no)
- Anything that affects deployment: (no)
### Documentation
- Does this pull request introduce a new feature? (yes)
- If yes, how is the feature documented? (not applicable / docs / JavaDocs
/ not documented)
- If a feature is not applicable for documentation, explain why?
- If a feature is not documented yet in this PR, please create a followup
issue for adding the documentation
--
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-unsubscr...@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
[GitHub] [pulsar] Anonymitaet commented on pull request #16865: [improve][workflow] Add a job in document bot to auto generate config docs
Anonymitaet commented on PR #16865: URL: https://github.com/apache/pulsar/pull/16865#issuecomment-1208849198 @tisonkun thanks for your reminder! We've discussed it and we're researching another way to implement it. For example, make [Pulsar configuration pages](https://pulsar.apache.org/docs/next/reference-configuration-broker) as independent pages (like https://pulsar.apache.org/tools/pulsar-admin/2.11.0-SNAPSHOT/) rather than `.md` files. In this way, we do not need to create such a workflow. -- 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-unsubscr...@pulsar.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [pulsar] tisonkun commented on issue #16912: Get rid of powermock
tisonkun commented on issue #16912: URL: https://github.com/apache/pulsar/issues/16912#issuecomment-1208848484 I draft an incompleted patch on #16997. However, I notice that there's some code we hack in "set final" field, like: https://github.com/apache/pulsar/blob/21634edd30bbe33d231ec73e8045d7666a8ad366/pulsar-broker/src/test/java/org/apache/pulsar/client/api/SimpleProducerConsumerTest.java#L1071-L1072 .. or even "set static final" field, like: https://github.com/apache/pulsar/blob/21634edd30bbe33d231ec73e8045d7666a8ad366/pulsar-functions/runtime/src/test/java/org/apache/pulsar/functions/runtime/thread/ThreadRuntimeFactoryTest.java#L111 It seems `powermock-reflect` is the better library existing to handle these cases since JDK12+ it's banned changing `final` field instead you have to use `Unsafe`. I tend not to maintain such unsafe hacks within Pulsar codebase. And perhaps we should gradually refactor code to avoid depending on such manners. cc @Shoothzj @eolivelli -- 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-unsubscr...@pulsar.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [pulsar] nodece commented on pull request #16765: [fix][flaky-test] Fix PulsarFunctionTlsTest.tearDown
nodece commented on PR #16765: URL: https://github.com/apache/pulsar/pull/16765#issuecomment-1208845851 Thank @codelipenghui for your report! I re-opened the above issue. -- 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-unsubscr...@pulsar.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [pulsar] michaeljmarshall opened a new pull request, #17002: Update .asf.yaml to protect branch-2.11
michaeljmarshall opened a new pull request, #17002: URL: https://github.com/apache/pulsar/pull/17002 ### Motivation See #14226 for original context. ### Modifications * Add `branch-2.11` to the list of protected branches ### Verifying this change This change is a trivial rework / code cleanup without any test coverage. ### Documentation - [x] `doc-not-needed` -- 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-unsubscr...@pulsar.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [pulsar] michaeljmarshall opened a new pull request, #17001: Fix swagger annotation for analyzeBacklog endpoint
michaeljmarshall opened a new pull request, #17001: URL: https://github.com/apache/pulsar/pull/17001 ### Motivation See https://github.com/apache/pulsar/pull/16222 for the motivation. ### Modifications * Fix an incorrect description for the `authoritative` swagger annotation. ### Documentation - [x] `doc` -- 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-unsubscr...@pulsar.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [pulsar] fantapsody commented on issue #9201: Saw "Unknown HK2 failure detected" Exception in the log when broker starts with function worker enabled
fantapsody commented on issue #9201: URL: https://github.com/apache/pulsar/issues/9201#issuecomment-1208840922 @heesung-sn During the container start K8s will access the `/status.html` API periodically to check if the broker is fully started, and the broker is considered to be started when [`PulsarService.start` finishes](https://github.com/apache/pulsar/blob/master/pulsar-broker/src/main/java/org/apache/pulsar/broker/PulsarService.java#L894). Before started, the status API will just throw not found exception: https://github.com/apache/pulsar/blob/master/pulsar-broker-common/src/main/java/org/apache/pulsar/common/configuration/VipStatus.java#L57 . So it should have no actual harm to the pulsar cluster. Maybe we can improve the status API to write some log like `Pulsar service is not ready` rather than throwing an exception directly. -- 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-unsubscr...@pulsar.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [pulsar] SignorMercurio closed pull request #16865: [improve][workflow] Add a job in document bot to auto generate config docs
SignorMercurio closed pull request #16865: [improve][workflow] Add a job in document bot to auto generate config docs URL: https://github.com/apache/pulsar/pull/16865 -- 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-unsubscr...@pulsar.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [pulsar-client-go] equanz commented on pull request #824: [cleanup] Fix to follow lint error (File is not `goimports`-ed (goimports))
equanz commented on PR #824: URL: https://github.com/apache/pulsar-client-go/pull/824#issuecomment-1208836215 https://github.com/apache/pulsar-client-go/pull/824#issuecomment-1208780551 [Succeeded](https://github.com/apache/pulsar-client-go/runs/7737458359?check_suite_focus=true). Thank you, @nkurihar . -- 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-unsubscr...@pulsar.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [pulsar] github-actions[bot] commented on issue #16497: Flaky-test: ManagedLedgerCompressionTest. testRestartBrokerEnableManagedLedgerInfoCompression
github-actions[bot] commented on issue #16497: URL: https://github.com/apache/pulsar/issues/16497#issuecomment-1208834819 The issue 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-unsubscr...@pulsar.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [pulsar] github-actions[bot] commented on issue #16498: Flaky-test: PersistentDispatcherFailoverConsumerTest. testAddRemoveConsumerNonPartitionedTopic
github-actions[bot] commented on issue #16498: URL: https://github.com/apache/pulsar/issues/16498#issuecomment-1208834805 The issue 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-unsubscr...@pulsar.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [pulsar] momo-jun commented on pull request #16891: [improve][doc] Add doc of reader config to `pulsar-io-debezium`
momo-jun commented on PR #16891: URL: https://github.com/apache/pulsar/pull/16891#issuecomment-1208830558 > > @freeznet thanks for adding the docs. I left several minor suggestions. Another concern is the historical doc versions you changed is 2.9.3 and 2.10.1 while the code PR seems to serve 2.9.4 and 2.10.2? > > @momo-jun you are right, could you please tell me how can I add the doc to 2.9.4 and 2.10.2? In the current practice of Pulsar doc release and maintenance, the docs for the upcoming 2.9.4 and 2.10.2 releases aren't in place until the release is ready to announce. Since [PIP-190](https://github.com/apache/pulsar/issues/16637) has been adopted, I'm generating 2.9.x and 2.10.x docs in #16925. You can leave them as they are in this PR and I will sync your changes there. -- 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-unsubscr...@pulsar.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[pulsar] branch master updated: [improve][doc] Improve TLS transport encryption and authentication (#16924)
This is an automated email from the ASF dual-hosted git repository.
zixuan pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/pulsar.git
The following commit(s) were added to refs/heads/master by this push:
new 21634edd30b [improve][doc] Improve TLS transport encryption and
authentication (#16924)
21634edd30b is described below
commit 21634edd30bbe33d231ec73e8045d7666a8ad366
Author: Zixuan Liu
AuthorDate: Tue Aug 9 10:06:00 2022 +0800
[improve][doc] Improve TLS transport encryption and authentication (#16924)
Signed-off-by: Zixuan Liu
---
site2/docs/security-tls-authentication.md | 15 +---
site2/docs/security-tls-keystore.md | 42 ++---
site2/docs/security-tls-transport.md | 62 ---
3 files changed, 87 insertions(+), 32 deletions(-)
diff --git a/site2/docs/security-tls-authentication.md
b/site2/docs/security-tls-authentication.md
index 295c1620dba..d82122dd7f1 100644
--- a/site2/docs/security-tls-authentication.md
+++ b/site2/docs/security-tls-authentication.md
@@ -8,8 +8,6 @@ sidebar_label: "Authentication using TLS"
TLS authentication is an extension of [TLS transport
encryption](security-tls-transport.md). Not only servers have keys and certs
that the client uses to verify the identity of servers, clients also have keys
and certs that the server uses to verify the identity of clients. You must have
TLS transport encryption configured on your cluster before you can use TLS
authentication. This guide assumes you already have TLS transport encryption
configured.
-`Bouncy Castle Provider` provides TLS related cipher suites and algorithms in
Pulsar. If you need [FIPS](https://www.bouncycastle.org/fips_faq.html) version
of `Bouncy Castle Provider`, please reference [Bouncy Castle
page](security-bouncy-castle.md).
-
### Create client certificates
Client certificates are generated using the certificate authority. Server
certificates are also generated with the same certificate authority.
@@ -87,11 +85,6 @@ To configure brokers to authenticate clients, add the
following parameters to `b
authenticationEnabled=true
authenticationProviders=org.apache.pulsar.broker.authentication.AuthenticationProviderTls
-# operations and publish/consume from all topics
-superUserRoles=admin
-
-# Authentication settings of the broker itself. Used when the broker connects
to other brokers, either in same or other clusters
-brokerClientTlsEnabled=true
brokerClientAuthenticationPlugin=org.apache.pulsar.client.impl.auth.AuthenticationTls
brokerClientAuthenticationParameters={"tlsCertFile":"/path/my-ca/admin.cert.pem","tlsKeyFile":"/path/my-ca/admin.key-pk8.pem"}
brokerClientTrustCertsFilePath=/path/my-ca/certs/ca.cert.pem
@@ -124,15 +117,10 @@ When you use TLS authentication, client connects via TLS
transport. You need to
[Command-line tools](reference-cli-tools.md) like
[`pulsar-admin`](/tools/pulsar-admin/),
[`pulsar-perf`](reference-cli-tools.md#pulsar-perf), and
[`pulsar-client`](reference-cli-tools.md#pulsar-client) use the
`conf/client.conf` config file in a Pulsar installation.
-You need to add the following parameters to that file to use TLS
authentication with the CLI tools of Pulsar:
+To use TLS authentication with the CLI tools of Pulsar, you need to add the
following parameters to the `conf/client.conf` file, alongside [the
configuration to enable TLS transport](security-tls-transport.md#cli-tools):
```properties
-webServiceUrl=https://broker.example.com:8443/
-brokerServiceUrl=pulsar+ssl://broker.example.com:6651/
-useTls=true
-tlsAllowInsecureConnection=false
-tlsTrustCertsFilePath=/path/to/ca.cert.pem
authPlugin=org.apache.pulsar.client.impl.auth.AuthenticationTls
authParams=tlsCertFile:/path/to/my-role.cert.pem,tlsKeyFile:/path/to/my-role.key-pk8.pem
@@ -146,7 +134,6 @@ import org.apache.pulsar.client.api.PulsarClient;
PulsarClient client = PulsarClient.builder()
.serviceUrl("pulsar+ssl://broker.example.com:6651/")
-.enableTls(true)
.tlsTrustCertsFilePath("/path/to/ca.cert.pem")
.authentication("org.apache.pulsar.client.impl.auth.AuthenticationTls",
"tlsCertFile:/path/to/my-role.cert.pem,tlsKeyFile:/path/to/my-role.key-pk8.pem")
diff --git a/site2/docs/security-tls-keystore.md
b/site2/docs/security-tls-keystore.md
index d655290a2cb..775415aab4b 100644
--- a/site2/docs/security-tls-keystore.md
+++ b/site2/docs/security-tls-keystore.md
@@ -85,6 +85,7 @@ The next step is to sign all certificates in the keystore
with the CA we generat
```shell
keytool -keystore broker.keystore.jks -alias localhost -certreq -file cert-file
+keytool -keystore client.keystore.jks -alias localhost -certreq -file cert-file
```
@@ -103,6 +104,8 @@ Finally, you need to import both the certificate of the CA
and the signed certif
keytool -keystore broker.keystore.jks -alias CARoot -import -file ca-cert
keytool -keystore
[GitHub] [pulsar] nodece merged pull request #16924: [improve][doc] Improve TLS transport encryption and authentication
nodece merged PR #16924: URL: https://github.com/apache/pulsar/pull/16924 -- 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-unsubscr...@pulsar.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [pulsar] heesung-sn commented on issue #9201: Saw "Unknown HK2 failure detected" Exception in the log when broker starts with function worker enabled
heesung-sn commented on issue #9201: URL: https://github.com/apache/pulsar/issues/9201#issuecomment-1208824043 Could you share the mitigation steps? How can we postpone the readiness probe? -- 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-unsubscr...@pulsar.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [pulsar] tisonkun commented on pull request #16902: [improve][repo] Tidy issue templates
tisonkun commented on PR #16902: URL: https://github.com/apache/pulsar/pull/16902#issuecomment-1208820226 Thanks for your reviews! Could you help with merging this patch? -- 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-unsubscr...@pulsar.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [pulsar] freeznet commented on pull request #16891: [improve][doc] Add doc of reader config to `pulsar-io-debezium`
freeznet commented on PR #16891: URL: https://github.com/apache/pulsar/pull/16891#issuecomment-1208819550 > @freeznet thanks for adding the docs. I left several minor suggestions. Another concern is the historical doc versions you changed is 2.9.3 and 2.10.1 while the code PR seems to serve 2.9.4 and 2.10.2? @momo-jun you are right, could you please tell me how can I add the doc to 2.9.4 and 2.10.2? -- 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-unsubscr...@pulsar.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [pulsar] mans2singh commented on pull request #16885: [fix][cookbooks-compaction][docs] - Updated consumer config section
mans2singh commented on PR #16885: URL: https://github.com/apache/pulsar/pull/16885#issuecomment-1208789005 Thanks @Anonymitaet for your time. -- 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-unsubscr...@pulsar.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[pulsar-site] branch main updated: [build]force build by empty commit: BUILD_ALL_VERSION=1 BUILD_ALL_LANGUAGE=1
This is an automated email from the ASF dual-hosted git repository. urfree pushed a commit to branch main in repository https://gitbox.apache.org/repos/asf/pulsar-site.git The following commit(s) were added to refs/heads/main by this push: new 3a733db45fb [build]force build by empty commit: BUILD_ALL_VERSION=1 BUILD_ALL_LANGUAGE=1 3a733db45fb is described below commit 3a733db45fbc8432cf884f6ce921a966984af60d Author: Li Li AuthorDate: Tue Aug 9 09:18:41 2022 +0800 [build]force build by empty commit: BUILD_ALL_VERSION=1 BUILD_ALL_LANGUAGE=1 Signed-off-by: Li Li
[pulsar] branch branch-2.9 updated: Fix MaxQueueSize semaphore release leak in createOpSendMsg (#16958)
This is an automated email from the ASF dual-hosted git repository.
technoboy pushed a commit to branch branch-2.9
in repository https://gitbox.apache.org/repos/asf/pulsar.git
The following commit(s) were added to refs/heads/branch-2.9 by this push:
new f8ce46e9fd9 Fix MaxQueueSize semaphore release leak in createOpSendMsg
(#16958)
f8ce46e9fd9 is described below
commit f8ce46e9fd918d78c1261eccc72ae384b2b1e9f4
Author: lixinyang <84127069+nicklee...@users.noreply.github.com>
AuthorDate: Tue Aug 9 09:10:52 2022 +0800
Fix MaxQueueSize semaphore release leak in createOpSendMsg (#16958)
---
.../pulsar/client/impl/ProducerSemaphoreTest.java | 33 ++
.../client/impl/BatchMessageContainerImpl.java | 1 +
2 files changed, 34 insertions(+)
diff --git
a/pulsar-broker/src/test/java/org/apache/pulsar/client/impl/ProducerSemaphoreTest.java
b/pulsar-broker/src/test/java/org/apache/pulsar/client/impl/ProducerSemaphoreTest.java
index 181e9d05d9c..de858c8d2bd 100644
---
a/pulsar-broker/src/test/java/org/apache/pulsar/client/impl/ProducerSemaphoreTest.java
+++
b/pulsar-broker/src/test/java/org/apache/pulsar/client/impl/ProducerSemaphoreTest.java
@@ -58,6 +58,39 @@ public class ProducerSemaphoreTest extends
ProducerConsumerBase {
super.internalCleanup();
}
+@Test(timeOut = 10_000)
+public void testProducerSemaphoreInvalidMessage() throws Exception {
+final int pendingQueueSize = 100;
+
+@Cleanup
+ProducerImpl producer = (ProducerImpl)
pulsarClient.newProducer()
+.topic("testProducerSemaphoreAcquire")
+.maxPendingMessages(pendingQueueSize)
+.enableBatching(true)
+.create();
+
+this.stopBroker();
+
+Field maxMessageSizeFiled =
ClientCnx.class.getDeclaredField("maxMessageSize");
+maxMessageSizeFiled.setAccessible(true);
+maxMessageSizeFiled.set(null, 2);
+
+try {
+producer.send("semaphore-test".getBytes(StandardCharsets.UTF_8));
+Assert.fail("can not reach here");
+} catch (PulsarClientException.InvalidMessageException ex) {
+
Assert.assertEquals(producer.getSemaphore().get().availablePermits(),
pendingQueueSize);
+}
+
+producer.conf.setBatchingEnabled(false);
+try {
+producer.send("semaphore-test".getBytes(StandardCharsets.UTF_8));
+Assert.fail("can not reach here");
+} catch (PulsarClientException.InvalidMessageException ex) {
+
Assert.assertEquals(producer.getSemaphore().get().availablePermits(),
pendingQueueSize);
+}
+}
+
@Test(timeOut = 3)
public void testProducerSemaphoreAcquireAndRelease() throws
PulsarClientException, ExecutionException, InterruptedException {
diff --git
a/pulsar-client/src/main/java/org/apache/pulsar/client/impl/BatchMessageContainerImpl.java
b/pulsar-client/src/main/java/org/apache/pulsar/client/impl/BatchMessageContainerImpl.java
index 996875a7131..e0ab2d942ca 100644
---
a/pulsar-client/src/main/java/org/apache/pulsar/client/impl/BatchMessageContainerImpl.java
+++
b/pulsar-client/src/main/java/org/apache/pulsar/client/impl/BatchMessageContainerImpl.java
@@ -199,6 +199,7 @@ class BatchMessageContainerImpl extends
AbstractBatchMessageContainer {
public OpSendMsg createOpSendMsg() throws IOException {
ByteBuf encryptedPayload = producer.encryptMessage(messageMetadata,
getCompressedBatchMetadataAndPayload());
if (encryptedPayload.readableBytes() > ClientCnx.getMaxMessageSize()) {
+producer.semaphoreRelease(messages.size());
discard(new PulsarClientException.InvalidMessageException(
"Message size is bigger than " +
ClientCnx.getMaxMessageSize() + " bytes"));
return null;
[GitHub] [pulsar] Technoboy- merged pull request #16958: [Branch-2.9][fix][client] Fix MaxQueueSize semaphore release leak in createOpSendMsg.
Technoboy- merged PR #16958: URL: https://github.com/apache/pulsar/pull/16958 -- 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-unsubscr...@pulsar.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [pulsar-client-go] equanz commented on pull request #824: [cleanup] Fix to follow lint error (File is not `goimports`-ed (goimports))
equanz commented on PR #824: URL: https://github.com/apache/pulsar-client-go/pull/824#issuecomment-1208780551 Could you retest this, please? I don't have a permission of running actions directly. And, I can't rerun failure tests by `/pulsarbot run-failure-checks`. ``` Run apache/pulsar-test-infra/pulsarbot@master /usr/bin/docker run --name cd98fdf476b76c8e64e80bfa865b6ee273c88_0dee37 --label 4cd98f --workdir /github/workspace --rm -e GITHUB_TOKEN -e HOME -e GITHUB_JOB -e GITHUB_REF -e GITHUB_SHA -e GITHUB_REPOSITORY -e GITHUB_REPOSITORY_OWNER -e GITHUB_RUN_ID -e GITHUB_RUN_NUMBER -e GITHUB_RETENTION_DAYS -e GITHUB_RUN_ATTEMPT -e GITHUB_ACTOR -e GITHUB_TRIGGERING_ACTOR -e GITHUB_WORKFLOW -e GITHUB_HEAD_REF -e GITHUB_BASE_REF -e GITHUB_EVENT_NAME -e GITHUB_SERVER_URL -e GITHUB_API_URL -e GITHUB_GRAPHQL_URL -e GITHUB_REF_NAME -e GITHUB_REF_PROTECTED -e GITHUB_REF_TYPE -e GITHUB_WORKSPACE -e GITHUB_ACTION -e GITHUB_EVENT_PATH -e GITHUB_ACTION_REPOSITORY -e GITHUB_ACTION_REF -e GITHUB_PATH -e GITHUB_ENV -e GITHUB_STEP_SUMMARY -e RUNNER_OS -e RUNNER_ARCH -e RUNNER_NAME -e RUNNER_TOOL_CACHE -e RUNNER_TEMP -e RUNNER_WORKSPACE -e ACTIONS_RUNTIME_URL -e ACTIONS_RUNTIME_TOKEN -e ACTIONS_CACHE_URL -e GITHUB_ACTIONS=true -e CI=true -v "/var/run/docker.sock":"/var/run/docker.sock" -v "/home/runner/work/ _temp/_github_home":"/github/home" -v "/home/runner/work/_temp/_github_workflow":"/github/workflow" -v "/home/runner/work/_temp/_runner_file_commands":"/github/file_commands" -v "/home/runner/work/pulsar-client-go/pulsar-client-go":"/github/workspace" 4cd98f:df476b76c8e64e80bfa865b6ee273c88 Handling pulsarbot command for PR #824 null jq: error (at :4): Cannot iterate over null (null) Cannot find any failed workflow runs in PR #824. Re-running can only target completed workflows. ``` https://github.com/apache/pulsar-client-go/runs/7737195510?check_suite_focus=true -- 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-unsubscr...@pulsar.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [pulsar-client-go] equanz commented on pull request #824: [cleanup] Fix to follow lint error (File is not `goimports`-ed (goimports))
equanz commented on PR #824: URL: https://github.com/apache/pulsar-client-go/pull/824#issuecomment-1208770316 /pulsarbot run integration-tests -- 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-unsubscr...@pulsar.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [pulsar-client-go] equanz commented on pull request #824: [cleanup] Fix to follow lint error (File is not `goimports`-ed (goimports))
equanz commented on PR #824: URL: https://github.com/apache/pulsar-client-go/pull/824#issuecomment-1208766204 /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-unsubscr...@pulsar.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [pulsar-client-go] equanz commented on pull request #824: [cleanup] Fix to follow lint error (File is not `goimports`-ed (goimports))
equanz commented on PR #824: URL: https://github.com/apache/pulsar-client-go/pull/824#issuecomment-1208764395 /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-unsubscr...@pulsar.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [pulsar-client-go] equanz opened a new pull request, #824: [cleanup] Fix to follow the lint error
equanz opened a new pull request, #824: URL: https://github.com/apache/pulsar-client-go/pull/824 ### Motivation Currently, the golangci-lint returns an error about coding style like below. ``` bash-4.2# /root/go/bin/golangci-lint run -v INFO [config_reader] Config search paths: [./ /pulsar-client-go / /root] INFO [config_reader] Used config file .golangci.yml INFO [lintersdb] Active 16 linters: [bodyclose deadcode goimports gosimple govet ineffassign lll misspell prealloc revive stylecheck typecheck unconvert unparam unused varcheck] INFO [loader] Go packages loading at mode 575 (compiled_files|types_sizes|deps|exports_file|files|imports|name) took 2.153604501s INFO [runner/filename_unadjuster] Pre-built 0 adjustments in 242.078666ms INFO [linters context/goanalysis] analyzers took 0s with no stages INFO [runner/skip dirs] Skipped 2 issues from dir examples/producer by pattern (^|/)examples($|/) INFO [runner] Issues before processing: 804, after processing: 1 INFO [runner] Processors filtering stat (out/in): severity-rules: 1/1, exclude-rules: 2/317, nolint: 1/2, uniq_by_line: 1/1, sort_results: 1/1, path_prettifier: 804/804, autogenerated_exclude: 317/802, identifier_marker: 317/317, skip_dirs: 802/804, exclude: 317/317, diff: 1/1, max_same_issues: 1/1, path_shortener: 1/1, cgo: 804/804, filename_unadjuster: 804/804, skip_files: 804/804, path_prefixer: 1/1, max_per_file_from_linter: 1/1, max_from_linter: 1/1, source_code: 1/1 INFO [runner] processing took 200.131536ms with stages: exclude-rules: 80.570624ms, autogenerated_exclude: 61.830875ms, identifier_marker: 41.972667ms, nolint: 7.293417ms, path_prettifier: 3.747624ms, skip_dirs: 1.673709ms, source_code: 1.1035ms, cgo: 502.291µs, severity-rules: 385.792µs, filename_unadjuster: 274.75µs, max_same_issues: 188.874µs, max_from_linter: 101.958µs, uniq_by_line: 93.333µs, path_shortener: 92.584µs, max_per_file_from_linter: 89.291µs, diff: 69.5µs, sort_results: 43.707µs, skip_files: 43.208µs, exclude: 33.958µs, path_prefixer: 19.874µs INFO [runner] linters took 563.266709ms with stages: goanalysis_metalinter: 361.755959ms pulsar/internal/commands.go:73: File is not `goimports`-ed (goimports) // INFO File cache stats: 1 entries of total size 10.0KiB INFO Memory: 32 samples, avg is 54.6MB, max is 83.1MB INFO Execution took 3.056773293s ``` ### Modifications Fix to follow the lint error. ### Verifying this change - [ ] Make sure that the change passes the CI checks. This change is a trivial rework / code cleanup without any test coverage. ### Does this pull request potentially affect one of the following parts: *If `yes` was chosen, please highlight the changes* - Dependencies (does it add or upgrade a dependency): (no) - The public API: (no) - The schema: (no) - The default values of configurations: (no) - The wire protocol: (no) ### Documentation - Does this pull request introduce a new feature? (no) -- 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-unsubscr...@pulsar.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[pulsar-site] 01/01: Merge pull request #158 from apache/fix-docs-page-crash
This is an automated email from the ASF dual-hosted git repository. urfree pushed a commit to branch main in repository https://gitbox.apache.org/repos/asf/pulsar-site.git commit 49731e5f78920ab4baf83677116d41514fc761c0 Merge: c3d49e992ca 70a1fbfc496 Author: Li Li AuthorDate: Tue Aug 9 08:20:43 2022 +0800 Merge pull request #158 from apache/fix-docs-page-crash drop the docusaurus version site2/website-next/package.json | 8 1 file changed, 4 insertions(+), 4 deletions(-)
[pulsar-site] 01/01: drop the docusaurus version
This is an automated email from the ASF dual-hosted git repository.
urfree pushed a commit to branch fix-docs-page-crash
in repository https://gitbox.apache.org/repos/asf/pulsar-site.git
commit 70a1fbfc496a8fc25eaaabd11a414d612624ec11
Author: Li Li
AuthorDate: Tue Aug 9 08:20:15 2022 +0800
drop the docusaurus version
Signed-off-by: Li Li
---
site2/website-next/package.json | 8
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/site2/website-next/package.json b/site2/website-next/package.json
index d2efc2671d9..f90380d7fdb 100644
--- a/site2/website-next/package.json
+++ b/site2/website-next/package.json
@@ -22,10 +22,10 @@
},
"dependencies": {
"@crowdin/cli": "3.7.4",
-"@docusaurus/core": "2.0.1",
-"@docusaurus/plugin-client-redirects": "2.0.1",
-"@docusaurus/plugin-google-analytics": "2.0.1",
-"@docusaurus/preset-classic": "2.0.1",
+"@docusaurus/core": "2.0.0-beta.21",
+"@docusaurus/plugin-client-redirects": "2.0.0-beta.21",
+"@docusaurus/plugin-google-analytics": "2.0.0-beta.21",
+"@docusaurus/preset-classic": "2.0.0-beta.21",
"@emotion/react": "^11.7.1",
"@emotion/styled": "^11.6.0",
"@mdx-js/react": "^1.6.22",
[pulsar-site] branch main updated (c3d49e992ca -> 49731e5f789)
This is an automated email from the ASF dual-hosted git repository. urfree pushed a change to branch main in repository https://gitbox.apache.org/repos/asf/pulsar-site.git from c3d49e992ca Docs sync done from apache/pulsar(#a88d952) add 70a1fbfc496 drop the docusaurus version new 49731e5f789 Merge pull request #158 from apache/fix-docs-page-crash The 1 revisions listed above as "new" are entirely new to this repository and will be described in separate emails. The revisions listed as "add" were already present in the repository and have only been added to this reference. Summary of changes: site2/website-next/package.json | 8 1 file changed, 4 insertions(+), 4 deletions(-)
[pulsar-site] branch fix-docs-page-crash created (now 70a1fbfc496)
This is an automated email from the ASF dual-hosted git repository. urfree pushed a change to branch fix-docs-page-crash in repository https://gitbox.apache.org/repos/asf/pulsar-site.git at 70a1fbfc496 drop the docusaurus version This branch includes the following new commits: new 70a1fbfc496 drop the docusaurus version The 1 revisions listed above as "new" are entirely new to this repository and will be described in separate emails. The revisions listed as "add" were already present in the repository and have only been added to this reference.
[pulsar-site] branch main updated: Docs sync done from apache/pulsar(#a88d952)
This is an automated email from the ASF dual-hosted git repository.
urfree pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/pulsar-site.git
The following commit(s) were added to refs/heads/main by this push:
new c3d49e992ca Docs sync done from apache/pulsar(#a88d952)
c3d49e992ca is described below
commit c3d49e992cabd45e1b242c7bd461988201c98aa2
Author: Pulsar Site Updater
AuthorDate: Tue Aug 9 00:01:54 2022 +
Docs sync done from apache/pulsar(#a88d952)
---
site2/website-next/static/swagger/restApiVersions.json | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/site2/website-next/static/swagger/restApiVersions.json
b/site2/website-next/static/swagger/restApiVersions.json
index b2292ab1572..8d35bd4d0b0 100644
--- a/site2/website-next/static/swagger/restApiVersions.json
+++ b/site2/website-next/static/swagger/restApiVersions.json
@@ -1,5 +1,5 @@
{
-"2.10.1": [
+"2.10.0": [
{
"version": "v2",
"fileName": [
@@ -16,7 +16,7 @@
]
}
],
-"2.10.0": [
+"2.10.1": [
{
"version": "v2",
"fileName": [
[GitHub] [pulsar] mattisonchao commented on pull request #16968: [fix][broker]Prevent `StackOverFlowException` in SHARED subscription
mattisonchao commented on PR #16968: URL: https://github.com/apache/pulsar/pull/16968#issuecomment-1208693033 Yes, this PR would fix this problem, but if the user disables this feature or in the old branch, we still need this PR. Plus, I will push a new cleanup PR to apply your comment. -- 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-unsubscr...@pulsar.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [pulsar] mattisonchao commented on a diff in pull request #16968: [fix][broker]Prevent `StackOverFlowException` in SHARED subscription
mattisonchao commented on code in PR #16968: URL: https://github.com/apache/pulsar/pull/16968#discussion_r940723127 ## pulsar-broker/src/main/java/org/apache/pulsar/broker/service/persistent/PersistentDispatcherMultipleConsumers.java: ## @@ -93,7 +93,7 @@ public class PersistentDispatcherMultipleConsumers extends AbstractDispatcherMul protected volatile PositionImpl minReplayedPosition = null; protected boolean shouldRewindBeforeReadingOrReplaying = false; protected final String name; -protected boolean sendInProgress; +protected volatile boolean sendInProgress; Review Comment: oh, sendInProgress will change in the `synchronized` block, -- 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-unsubscr...@pulsar.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [pulsar] mattisonchao commented on a diff in pull request #16968: [fix][broker]Prevent `StackOverFlowException` in SHARED subscription
mattisonchao commented on code in PR #16968:
URL: https://github.com/apache/pulsar/pull/16968#discussion_r940722330
##
pulsar-broker/src/main/java/org/apache/pulsar/broker/service/persistent/PersistentDispatcherMultipleConsumers.java:
##
@@ -544,24 +550,25 @@ public final synchronized void
readEntriesComplete(List entries, Object c
if (serviceConfig.isDispatcherDispatchMessagesInSubscriptionThread()) {
// setting sendInProgress here, because sendMessagesToConsumers
will be executed
// in a separate thread, and we want to prevent more reads
Review Comment:
It's better to move it to line 565.
--
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-unsubscr...@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
[GitHub] [pulsar] poorbarcode commented on a diff in pull request #16758: [improve][txn] PIP-160 Metrics stats of Transaction buffered writer
poorbarcode commented on code in PR #16758:
URL: https://github.com/apache/pulsar/pull/16758#discussion_r940618722
##
pulsar-transaction/coordinator/src/main/java/org/apache/pulsar/transaction/coordinator/impl/TxnLogBufferedWriter.java:
##
@@ -198,33 +209,99 @@ public void asyncAddData(T data, AddDataCallback
callback, Object ctx){
AsyncAddArgs.newInstance(callback, ctx,
System.currentTimeMillis(), byteBuf));
return;
}
-singleThreadExecutorForWrite.execute(() -> internalAsyncAddData(data,
callback, ctx));
+singleThreadExecutorForWrite.execute(() -> {
+try {
+internalAsyncAddData(data, callback, ctx);
+} catch (Exception e){
+log.error("Internal async add data fail", e);
Review Comment:
> But it will break the order, no? If you have 3 records need to write to
the managed ledger, A, B, and C. If A fails here, B and C will continue to
write. As you said the request hold in the memory queue, the next round A will
be retry.
If A already append to the array, it should be ok.
I added a logic that fails callback if the append to the queue fails
> As you said the request hold in the memory queue, the next round A will be
retry.
No, If A is put into the queue successfully, A will be called back earlier
than B and C
--
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-unsubscr...@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
[GitHub] [pulsar] poorbarcode commented on a diff in pull request #16758: [improve][txn] PIP-160 Metrics stats of Transaction buffered writer
poorbarcode commented on code in PR #16758:
URL: https://github.com/apache/pulsar/pull/16758#discussion_r940618722
##
pulsar-transaction/coordinator/src/main/java/org/apache/pulsar/transaction/coordinator/impl/TxnLogBufferedWriter.java:
##
@@ -198,33 +209,99 @@ public void asyncAddData(T data, AddDataCallback
callback, Object ctx){
AsyncAddArgs.newInstance(callback, ctx,
System.currentTimeMillis(), byteBuf));
return;
}
-singleThreadExecutorForWrite.execute(() -> internalAsyncAddData(data,
callback, ctx));
+singleThreadExecutorForWrite.execute(() -> {
+try {
+internalAsyncAddData(data, callback, ctx);
+} catch (Exception e){
+log.error("Internal async add data fail", e);
Review Comment:
> But it will break the order, no? If you have 3 records need to write to
the managed ledger, A, B, and C. If A fails here, B and C will continue to
write. As you said the request hold in the memory queue, the next round A will
be retry.
If A already append to the array, it should be ok.
I added a logic that fails callback if the append to the queue fails
--
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-unsubscr...@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
[GitHub] [pulsar] poorbarcode commented on a diff in pull request #16758: [improve][txn] PIP-160 Metrics stats of Transaction buffered writer
poorbarcode commented on code in PR #16758:
URL: https://github.com/apache/pulsar/pull/16758#discussion_r940608784
##
pulsar-transaction/coordinator/src/main/java/org/apache/pulsar/transaction/coordinator/impl/TxnLogBufferedWriter.java:
##
@@ -198,33 +209,99 @@ public void asyncAddData(T data, AddDataCallback
callback, Object ctx){
AsyncAddArgs.newInstance(callback, ctx,
System.currentTimeMillis(), byteBuf));
return;
}
-singleThreadExecutorForWrite.execute(() -> internalAsyncAddData(data,
callback, ctx));
+singleThreadExecutorForWrite.execute(() -> {
+try {
+internalAsyncAddData(data, callback, ctx);
+} catch (Exception e){
+log.error("Internal async add data fail", e);
+}
+});
}
+/**
+ * Append data to queue, if reach {@link #batchedWriteMaxRecords} or
{@link #batchedWriteMaxSize}, do flush. And if
+ * accept a request that {@param data} is too large (larger than {@link
#batchedWriteMaxSize}), then two flushes
+ * are executed:
+ *1. Write the data cached in the queue to BK.
+ *2. Direct write the large data to BK, this flush event will not
record to Metrics.
+ * This ensures the sequential nature of multiple writes to BK.
+ */
private void internalAsyncAddData(T data, AddDataCallback callback, Object
ctx){
if (state == State.CLOSING || state == State.CLOSED){
callback.addFailed(BUFFERED_WRITER_CLOSED_EXCEPTION, ctx);
return;
}
-int len = dataSerializer.getSerializedSize(data);
-if (len >= batchedWriteMaxSize){
-if (!flushContext.asyncAddArgsList.isEmpty()) {
-doTrigFlush(true, false);
-}
+int dataLength = dataSerializer.getSerializedSize(data);
+if (dataLength >= batchedWriteMaxSize){
+trigFlushByLargeSingleData();
ByteBuf byteBuf = dataSerializer.serialize(data);
managedLedger.asyncAddEntry(byteBuf,
DisabledBatchCallback.INSTANCE,
AsyncAddArgs.newInstance(callback, ctx,
System.currentTimeMillis(), byteBuf));
return;
}
-// Add data.
-this.dataArray.add(data);
-// Add callback info.
+// Append data to the data-array.
+dataArray.add(data);
+// Append callback to the flushContext.
AsyncAddArgs asyncAddArgs = AsyncAddArgs.newInstance(callback, ctx,
System.currentTimeMillis());
-this.flushContext.asyncAddArgsList.add(asyncAddArgs);
-// Calculate bytes-size.
-this.bytesSize += len;
-// trig flush.
-doTrigFlush(false, false);
+flushContext.asyncAddArgsList.add(asyncAddArgs);
+// Calculate bytes size.
+bytesSize += dataLength;
+trigFlushIfReachMaxRecordsOrMaxSize();
+}
+
+/**
+ * Change to IO thread and do flush, only called by {@link
#timingFlushTask}.
+ */
+private void trigFlushByTimingTask(){
+singleThreadExecutorForWrite.execute(() -> {
+try {
+if (flushContext.asyncAddArgsList.isEmpty()) {
+return;
+}
+if (metrics != null) {
+
metrics.triggerFlushByByMaxDelay(flushContext.asyncAddArgsList.size(),
bytesSize,
+System.currentTimeMillis() -
flushContext.asyncAddArgsList.get(0).addedTime);
+}
+doFlush();
+} catch (Exception e){
+log.error("Trig flush by timing task fail.", e);
Review Comment:
> Same as the above comment
`doFlush` failure will not break the order, there is no problem 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-unsubscr...@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
[GitHub] [pulsar] poorbarcode commented on a diff in pull request #16758: [improve][txn] PIP-160 Metrics stats of Transaction buffered writer
poorbarcode commented on code in PR #16758:
URL: https://github.com/apache/pulsar/pull/16758#discussion_r940601418
##
pulsar-transaction/coordinator/src/main/java/org/apache/pulsar/transaction/coordinator/impl/TxnLogBufferedWriter.java:
##
@@ -198,33 +209,99 @@ public void asyncAddData(T data, AddDataCallback
callback, Object ctx){
AsyncAddArgs.newInstance(callback, ctx,
System.currentTimeMillis(), byteBuf));
return;
}
-singleThreadExecutorForWrite.execute(() -> internalAsyncAddData(data,
callback, ctx));
+singleThreadExecutorForWrite.execute(() -> {
+try {
+internalAsyncAddData(data, callback, ctx);
+} catch (Exception e){
+log.error("Internal async add data fail", e);
+}
+});
}
+/**
+ * Append data to queue, if reach {@link #batchedWriteMaxRecords} or
{@link #batchedWriteMaxSize}, do flush. And if
+ * accept a request that {@param data} is too large (larger than {@link
#batchedWriteMaxSize}), then two flushes
+ * are executed:
+ *1. Write the data cached in the queue to BK.
+ *2. Direct write the large data to BK, this flush event will not
record to Metrics.
+ * This ensures the sequential nature of multiple writes to BK.
+ */
private void internalAsyncAddData(T data, AddDataCallback callback, Object
ctx){
if (state == State.CLOSING || state == State.CLOSED){
callback.addFailed(BUFFERED_WRITER_CLOSED_EXCEPTION, ctx);
return;
}
-int len = dataSerializer.getSerializedSize(data);
-if (len >= batchedWriteMaxSize){
-if (!flushContext.asyncAddArgsList.isEmpty()) {
-doTrigFlush(true, false);
-}
+int dataLength = dataSerializer.getSerializedSize(data);
+if (dataLength >= batchedWriteMaxSize){
+trigFlushByLargeSingleData();
ByteBuf byteBuf = dataSerializer.serialize(data);
managedLedger.asyncAddEntry(byteBuf,
DisabledBatchCallback.INSTANCE,
AsyncAddArgs.newInstance(callback, ctx,
System.currentTimeMillis(), byteBuf));
return;
}
-// Add data.
-this.dataArray.add(data);
-// Add callback info.
+// Append data to the data-array.
+dataArray.add(data);
+// Append callback to the flushContext.
AsyncAddArgs asyncAddArgs = AsyncAddArgs.newInstance(callback, ctx,
System.currentTimeMillis());
-this.flushContext.asyncAddArgsList.add(asyncAddArgs);
-// Calculate bytes-size.
-this.bytesSize += len;
-// trig flush.
-doTrigFlush(false, false);
+flushContext.asyncAddArgsList.add(asyncAddArgs);
+// Calculate bytes size.
+bytesSize += dataLength;
+trigFlushIfReachMaxRecordsOrMaxSize();
+}
+
+/**
+ * Change to IO thread and do flush, only called by {@link
#timingFlushTask}.
+ */
+private void trigFlushByTimingTask(){
+singleThreadExecutorForWrite.execute(() -> {
+try {
+if (flushContext.asyncAddArgsList.isEmpty()) {
+return;
+}
+if (metrics != null) {
+
metrics.triggerFlushByByMaxDelay(flushContext.asyncAddArgsList.size(),
bytesSize,
+System.currentTimeMillis() -
flushContext.asyncAddArgsList.get(0).addedTime);
+}
+doFlush();
+} catch (Exception e){
+log.error("Trig flush by timing task fail.", e);
+} finally {
+// Start the next timing task.
+nextTimingTrigger();
Review Comment:
> Can't you just create your own single thread executor service which is
ScheduledExecutorService and use it for the scheduling of period flush instead
of implementing it on your own using Timer?
Using 'ScheduledExecutorService' could not make code less, because we can't
use the method `scheduleWithFixedDelay`. PR #16679 explains why.
We're using a `Timer` just to reuse objects that already exist.
PR 16679 focuses on the problem: avoiding a large number of "fixed-time
flush" tasks in the IO thread
--
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-unsubscr...@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
[GitHub] [pulsar] poorbarcode commented on a diff in pull request #16758: [improve][txn] PIP-160 Metrics stats of Transaction buffered writer
poorbarcode commented on code in PR #16758:
URL: https://github.com/apache/pulsar/pull/16758#discussion_r940594431
##
pulsar-transaction/coordinator/src/main/java/org/apache/pulsar/transaction/coordinator/impl/TxnLogBufferedWriter.java:
##
@@ -198,33 +209,99 @@ public void asyncAddData(T data, AddDataCallback
callback, Object ctx){
AsyncAddArgs.newInstance(callback, ctx,
System.currentTimeMillis(), byteBuf));
return;
}
-singleThreadExecutorForWrite.execute(() -> internalAsyncAddData(data,
callback, ctx));
+singleThreadExecutorForWrite.execute(() -> {
+try {
+internalAsyncAddData(data, callback, ctx);
+} catch (Exception e){
+log.error("Internal async add data fail", e);
+}
+});
}
+/**
+ * Append data to queue, if reach {@link #batchedWriteMaxRecords} or
{@link #batchedWriteMaxSize}, do flush. And if
+ * accept a request that {@param data} is too large (larger than {@link
#batchedWriteMaxSize}), then two flushes
+ * are executed:
+ *1. Write the data cached in the queue to BK.
+ *2. Direct write the large data to BK, this flush event will not
record to Metrics.
+ * This ensures the sequential nature of multiple writes to BK.
+ */
private void internalAsyncAddData(T data, AddDataCallback callback, Object
ctx){
if (state == State.CLOSING || state == State.CLOSED){
callback.addFailed(BUFFERED_WRITER_CLOSED_EXCEPTION, ctx);
return;
}
-int len = dataSerializer.getSerializedSize(data);
-if (len >= batchedWriteMaxSize){
-if (!flushContext.asyncAddArgsList.isEmpty()) {
-doTrigFlush(true, false);
-}
+int dataLength = dataSerializer.getSerializedSize(data);
+if (dataLength >= batchedWriteMaxSize){
+trigFlushByLargeSingleData();
ByteBuf byteBuf = dataSerializer.serialize(data);
managedLedger.asyncAddEntry(byteBuf,
DisabledBatchCallback.INSTANCE,
AsyncAddArgs.newInstance(callback, ctx,
System.currentTimeMillis(), byteBuf));
return;
}
-// Add data.
-this.dataArray.add(data);
-// Add callback info.
+// Append data to the data-array.
+dataArray.add(data);
+// Append callback to the flushContext.
AsyncAddArgs asyncAddArgs = AsyncAddArgs.newInstance(callback, ctx,
System.currentTimeMillis());
-this.flushContext.asyncAddArgsList.add(asyncAddArgs);
-// Calculate bytes-size.
-this.bytesSize += len;
-// trig flush.
-doTrigFlush(false, false);
+flushContext.asyncAddArgsList.add(asyncAddArgs);
+// Calculate bytes size.
+bytesSize += dataLength;
+trigFlushIfReachMaxRecordsOrMaxSize();
+}
+
+/**
+ * Change to IO thread and do flush, only called by {@link
#timingFlushTask}.
+ */
+private void trigFlushByTimingTask(){
+singleThreadExecutorForWrite.execute(() -> {
+try {
+if (flushContext.asyncAddArgsList.isEmpty()) {
+return;
+}
+if (metrics != null) {
+
metrics.triggerFlushByByMaxDelay(flushContext.asyncAddArgsList.size(),
bytesSize,
+System.currentTimeMillis() -
flushContext.asyncAddArgsList.get(0).addedTime);
+}
+doFlush();
+} catch (Exception e){
+log.error("Trig flush by timing task fail.", e);
+} finally {
+// Start the next timing task.
+nextTimingTrigger();
+}
+});
+}
+
+/**
+ * If reach the thresholds {@link #batchedWriteMaxRecords} or {@link
#batchedWriteMaxSize}, do flush.
+ */
+private void trigFlushIfReachMaxRecordsOrMaxSize(){
+if (flushContext.asyncAddArgsList.size() >= batchedWriteMaxRecords) {
Review Comment:
> What you wrote is there two 2 separate threads using asyncAddArgsList no?
While the second thread holds the reference to `asyncAddArgsList `, the
first thread has lost the reference to `asyncAddArgsList `, so there will never
be concurrent access
--
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-unsubscr...@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
[GitHub] [pulsar] poorbarcode commented on a diff in pull request #16758: [improve][txn] PIP-160 Metrics stats of Transaction buffered writer
poorbarcode commented on code in PR #16758:
URL: https://github.com/apache/pulsar/pull/16758#discussion_r940592357
##
pulsar-transaction/coordinator/src/main/java/org/apache/pulsar/transaction/coordinator/impl/TxnLogBufferedWriter.java:
##
@@ -259,64 +336,24 @@ private void internalAsyncAddData(T data, AddDataCallback
callback, Object ctx){
}
-/**
- * Trigger write to bookie once, If the conditions are not met, nothing
will be done.
- */
-public void trigFlush(final boolean force, boolean byScheduleThreads){
-singleThreadExecutorForWrite.execute(() -> doTrigFlush(force,
byScheduleThreads));
-}
-
-private void doTrigFlush(boolean force, boolean byScheduleThreads){
-try {
-if (flushContext.asyncAddArgsList.isEmpty()) {
-return;
-}
-if (force) {
-doFlush();
-return;
-}
-if (byScheduleThreads) {
-doFlush();
-return;
-}
-AsyncAddArgs firstAsyncAddArgs =
flushContext.asyncAddArgsList.get(0);
-if (System.currentTimeMillis() - firstAsyncAddArgs.addedTime >=
batchedWriteMaxDelayInMillis) {
-doFlush();
-return;
-}
-if (this.flushContext.asyncAddArgsList.size() >=
batchedWriteMaxRecords) {
-doFlush();
-return;
-}
-if (this.bytesSize >= batchedWriteMaxSize) {
-doFlush();
-}
-} finally {
-if (byScheduleThreads) {
-nextTimingTrigger();
-}
-}
-}
-
private void doFlush(){
-// Combine data.
-ByteBuf prefix = PulsarByteBufAllocator.DEFAULT.buffer(4);
-prefix.writeShort(BATCHED_ENTRY_DATA_PREFIX_MAGIC_NUMBER);
-prefix.writeShort(BATCHED_ENTRY_DATA_PREFIX_VERSION);
-ByteBuf actualContent = this.dataSerializer.serialize(this.dataArray);
-ByteBuf pairByteBuf = Unpooled.wrappedUnmodifiableBuffer(prefix,
actualContent);
+// Combine data cached by flushContext, and write to BK.
+ByteBuf prefixByteFuf = PulsarByteBufAllocator.DEFAULT.buffer(4);
+prefixByteFuf.writeShort(BATCHED_ENTRY_DATA_PREFIX_MAGIC_NUMBER);
+prefixByteFuf.writeShort(BATCHED_ENTRY_DATA_PREFIX_VERSION);
+ByteBuf contentByteBuf = dataSerializer.serialize(dataArray);
+ByteBuf wholeByteBuf =
Unpooled.wrappedUnmodifiableBuffer(prefixByteFuf, contentByteBuf);
Review Comment:
> So you do get a composite bytebuf, because you want to avoid the copy of
course.
'FixedCompositeByteBuf' and 'CompositeByteBuf' are similar names, but they
have no relation. `FixedCompositeByteBuf` is a just value object, there is only
the overhead of creating the 'FixedCompositeByteBuf$Component' object and the
GC overhead, is appropriate for this scenario.
> When you're not using the pulsar version you're missing out on several
configurations that might come in handy.
I could not understand
--
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-unsubscr...@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
[GitHub] [pulsar] poorbarcode commented on a diff in pull request #16758: [improve][txn] PIP-160 Metrics stats of Transaction buffered writer
poorbarcode commented on code in PR #16758:
URL: https://github.com/apache/pulsar/pull/16758#discussion_r940585431
##
pulsar-transaction/coordinator/src/main/java/org/apache/pulsar/transaction/coordinator/impl/TxnLogBufferedWriter.java:
##
@@ -259,64 +336,24 @@ private void internalAsyncAddData(T data, AddDataCallback
callback, Object ctx){
}
-/**
- * Trigger write to bookie once, If the conditions are not met, nothing
will be done.
- */
-public void trigFlush(final boolean force, boolean byScheduleThreads){
-singleThreadExecutorForWrite.execute(() -> doTrigFlush(force,
byScheduleThreads));
-}
-
-private void doTrigFlush(boolean force, boolean byScheduleThreads){
-try {
-if (flushContext.asyncAddArgsList.isEmpty()) {
-return;
-}
-if (force) {
-doFlush();
-return;
-}
-if (byScheduleThreads) {
-doFlush();
-return;
-}
-AsyncAddArgs firstAsyncAddArgs =
flushContext.asyncAddArgsList.get(0);
-if (System.currentTimeMillis() - firstAsyncAddArgs.addedTime >=
batchedWriteMaxDelayInMillis) {
-doFlush();
-return;
-}
-if (this.flushContext.asyncAddArgsList.size() >=
batchedWriteMaxRecords) {
-doFlush();
-return;
-}
-if (this.bytesSize >= batchedWriteMaxSize) {
-doFlush();
-}
-} finally {
-if (byScheduleThreads) {
-nextTimingTrigger();
-}
-}
-}
-
private void doFlush(){
-// Combine data.
-ByteBuf prefix = PulsarByteBufAllocator.DEFAULT.buffer(4);
-prefix.writeShort(BATCHED_ENTRY_DATA_PREFIX_MAGIC_NUMBER);
-prefix.writeShort(BATCHED_ENTRY_DATA_PREFIX_VERSION);
-ByteBuf actualContent = this.dataSerializer.serialize(this.dataArray);
-ByteBuf pairByteBuf = Unpooled.wrappedUnmodifiableBuffer(prefix,
actualContent);
+// Combine data cached by flushContext, and write to BK.
+ByteBuf prefixByteFuf = PulsarByteBufAllocator.DEFAULT.buffer(4);
+prefixByteFuf.writeShort(BATCHED_ENTRY_DATA_PREFIX_MAGIC_NUMBER);
+prefixByteFuf.writeShort(BATCHED_ENTRY_DATA_PREFIX_VERSION);
+ByteBuf contentByteBuf = dataSerializer.serialize(dataArray);
+ByteBuf wholeByteBuf =
Unpooled.wrappedUnmodifiableBuffer(prefixByteFuf, contentByteBuf);
// We need to release this pairByteBuf after Managed ledger async add
callback. Just holds by FlushContext.
Review Comment:
Yes, bad comments can be misleading. I have understood it and I will
gradually establish good habits
--
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-unsubscr...@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
[GitHub] [pulsar] poorbarcode commented on a diff in pull request #16758: [improve][txn] PIP-160 Metrics stats of Transaction buffered writer
poorbarcode commented on code in PR #16758:
URL: https://github.com/apache/pulsar/pull/16758#discussion_r940584089
##
pulsar-transaction/coordinator/src/test/java/org/apache/pulsar/transaction/coordinator/impl/TxnLogBufferedWriterTest.java:
##
@@ -613,9 +610,371 @@ public ByteBuf serialize(ArrayList dataArray) {
}
}
+private static class RandomLenSumStrDataSerializer extends
SumStrDataSerializer {
+
+@Getter
+private int totalSize;
+
+/**
+ * After the test, when {@link
TxnLogBufferedWriterConfig#getBatchedWriteMaxRecords()} = 256
+ * and {@link TxnLogBufferedWriterConfig#getBatchedWriteMaxSize()} =
1024,
+ * and {@link
TxnLogBufferedWriterConfig#getBatchedWriteMaxDelayInMillis()} = 1, the
random-size baseline
+ * was set as 9, and there was maximum probability that all three
thresholds could be hit.
+ */
+@Override
+public int getSerializedSize(Integer data) {
+int size = new Random().nextInt(9);
+totalSize += size;
+return size;
+}
+}
+
+private static class TwoLenSumDataSerializer extends JsonDataSerializer {
+
+private final int len1;
+
+private final int len2;
+
+private AtomicBoolean useLen2 = new AtomicBoolean();
+
+public TwoLenSumDataSerializer(int len1, int len2){
+this.len1 = len1;
+this.len2 = len2;
+}
+
+/**
+ * After the test, when {@link
TxnLogBufferedWriterConfig#getBatchedWriteMaxRecords()} = 256
+ * and {@link TxnLogBufferedWriterConfig#getBatchedWriteMaxSize()} =
1024,
+ * and {@link
TxnLogBufferedWriterConfig#getBatchedWriteMaxDelayInMillis()} = 1, the
random-size baseline
+ * was set as 9, and there was maximum probability that all three
thresholds could be hit.
+ */
+@Override
+public int getSerializedSize(Integer data) {
+boolean b = useLen2.get();
+useLen2.set(!b);
+return b ? len2 : len1;
+}
+}
+
public enum BookieErrorType{
NO_ERROR,
ALWAYS_ERROR,
SOMETIMES_ERROR;
}
+
+/**
+ * Test Transaction buffered writer stats when disabled batch feature.
+ */
+@Test
+public void testMetricsStatsWhenDisabledBatchFeature() throws Exception {
+TxnLogBufferedWriterMetricsStats metricsStats = new
TxnLogBufferedWriterMetricsStats(
+metricsPrefix, metricsLabelNames, metricsLabelValues,
CollectorRegistry.defaultRegistry
+);
+ManagedLedger managedLedger = factory.open("tx_test_ledger");
+// Create callback with counter.
+var callbackWithCounter = createCallBackWithCounter();
+OrderedExecutor orderedExecutor =
OrderedExecutor.newBuilder().numThreads(5).name("txn-threads").build();
+// Create TxnLogBufferedWriter.
+HashedWheelTimer transactionTimer = new HashedWheelTimer(new
DefaultThreadFactory("transaction-timer"),
+1, TimeUnit.MILLISECONDS);
+var dataSerializer = new RandomLenSumStrDataSerializer();
+var txnLogBufferedWriter = new TxnLogBufferedWriter(
+managedLedger, orderedExecutor, transactionTimer,
+dataSerializer, Integer.MAX_VALUE, Integer.MAX_VALUE,
+Integer.MAX_VALUE, false, metricsStats);
+// Add some data.
+int writeCount = 1000;
+for (int i = 0; i < writeCount; i++){
+txnLogBufferedWriter.asyncAddData(1, callbackWithCounter.callback,
"");
+}
+// Wait for all data write finish.
+Awaitility.await().atMost(2, TimeUnit.SECONDS).until(
+() -> callbackWithCounter.finishCounter.get() +
callbackWithCounter.failureCounter.get() == writeCount
+);
+assertEquals(callbackWithCounter.failureCounter.get(), 0);
+// Assert metrics stat.
+verifyTheHistogramMetrics(0, 0, 0);
+// cleanup.
+txnLogBufferedWriter.close();
+metricsStats.close();
+transactionTimer.stop();
+orderedExecutor.shutdown();
+}
+
+@Test
+public void testMetricsStatsThatTriggeredByMaxRecordCount() throws
Exception {
+SumStrDataSerializer dataSerializer = new SumStrDataSerializer();
+int batchedWriteMaxRecords = 2;
+int writeCount = 100;
+int expectedBatchFlushCount = writeCount / batchedWriteMaxRecords;
+int expectedTotalBytesSize = writeCount *
dataSerializer.getSizePerData();
+// Create callback with counter.
+var callbackWithCounter = createCallBackWithCounter();
+// Create TxnLogBufferedWriter.
+var txnLogBufferedWriterContext =
createTxnBufferedWriterContextWithMetrics(
+dataSerializer, batchedWriteMaxRecords, Integer.MAX_VALUE,
Integer.MAX_VALUE);
+var txnLogBufferedWriter =
[GitHub] [pulsar] poorbarcode commented on a diff in pull request #16758: [improve][txn] PIP-160 Metrics stats of Transaction buffered writer
poorbarcode commented on code in PR #16758:
URL: https://github.com/apache/pulsar/pull/16758#discussion_r940573523
##
pulsar-transaction/coordinator/src/main/java/org/apache/pulsar/transaction/coordinator/impl/TxnLogBufferedWriter.java:
##
@@ -541,8 +539,53 @@ public void recycle(){
this.asyncAddArgsList.clear();
this.handle.recycle(this);
}
+
+public void addCallback(AddDataCallback callback, Object ctx){
+AsyncAddArgs asyncAddArgs = AsyncAddArgs.newInstance(callback,
ctx, System.currentTimeMillis());
+asyncAddArgsList.add(asyncAddArgs);
+}
}
+/** Callback for batch write BK. **/
+private final BufferedAddEntryCallback bufferedAddEntryCallback = new
BufferedAddEntryCallback();
Review Comment:
Thank you for your example, already fixed
--
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-unsubscr...@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
[GitHub] [pulsar] poorbarcode commented on a diff in pull request #16758: [improve][txn] PIP-160 Metrics stats of Transaction buffered writer
poorbarcode commented on code in PR #16758:
URL: https://github.com/apache/pulsar/pull/16758#discussion_r940573061
##
pulsar-transaction/coordinator/src/test/java/org/apache/pulsar/transaction/coordinator/impl/TxnLogBufferedWriterTest.java:
##
@@ -613,9 +610,371 @@ public ByteBuf serialize(ArrayList dataArray) {
}
}
+private static class RandomLenSumStrDataSerializer extends
SumStrDataSerializer {
+
+@Getter
+private int totalSize;
+
+/**
+ * After the test, when {@link
TxnLogBufferedWriterConfig#getBatchedWriteMaxRecords()} = 256
+ * and {@link TxnLogBufferedWriterConfig#getBatchedWriteMaxSize()} =
1024,
+ * and {@link
TxnLogBufferedWriterConfig#getBatchedWriteMaxDelayInMillis()} = 1, the
random-size baseline
+ * was set as 9, and there was maximum probability that all three
thresholds could be hit.
+ */
+@Override
+public int getSerializedSize(Integer data) {
+int size = new Random().nextInt(9);
+totalSize += size;
+return size;
+}
+}
+
+private static class TwoLenSumDataSerializer extends JsonDataSerializer {
+
+private final int len1;
+
+private final int len2;
+
+private AtomicBoolean useLen2 = new AtomicBoolean();
+
+public TwoLenSumDataSerializer(int len1, int len2){
+this.len1 = len1;
+this.len2 = len2;
+}
+
+/**
+ * After the test, when {@link
TxnLogBufferedWriterConfig#getBatchedWriteMaxRecords()} = 256
+ * and {@link TxnLogBufferedWriterConfig#getBatchedWriteMaxSize()} =
1024,
+ * and {@link
TxnLogBufferedWriterConfig#getBatchedWriteMaxDelayInMillis()} = 1, the
random-size baseline
+ * was set as 9, and there was maximum probability that all three
thresholds could be hit.
+ */
+@Override
+public int getSerializedSize(Integer data) {
+boolean b = useLen2.get();
+useLen2.set(!b);
+return b ? len2 : len1;
+}
+}
+
public enum BookieErrorType{
NO_ERROR,
ALWAYS_ERROR,
SOMETIMES_ERROR;
}
+
+/**
+ * Test Transaction buffered writer stats when disabled batch feature.
+ */
+@Test
+public void testMetricsStatsWhenDisabledBatchFeature() throws Exception {
+TxnLogBufferedWriterMetricsStats metricsStats = new
TxnLogBufferedWriterMetricsStats(
+metricsPrefix, metricsLabelNames, metricsLabelValues,
CollectorRegistry.defaultRegistry
+);
+ManagedLedger managedLedger = factory.open("tx_test_ledger");
+// Create callback with counter.
+var callbackWithCounter = createCallBackWithCounter();
+OrderedExecutor orderedExecutor =
OrderedExecutor.newBuilder().numThreads(5).name("txn-threads").build();
+// Create TxnLogBufferedWriter.
+HashedWheelTimer transactionTimer = new HashedWheelTimer(new
DefaultThreadFactory("transaction-timer"),
+1, TimeUnit.MILLISECONDS);
+var dataSerializer = new RandomLenSumStrDataSerializer();
+var txnLogBufferedWriter = new TxnLogBufferedWriter(
+managedLedger, orderedExecutor, transactionTimer,
+dataSerializer, Integer.MAX_VALUE, Integer.MAX_VALUE,
+Integer.MAX_VALUE, false, metricsStats);
+// Add some data.
+int writeCount = 1000;
+for (int i = 0; i < writeCount; i++){
+txnLogBufferedWriter.asyncAddData(1, callbackWithCounter.callback,
"");
+}
+// Wait for all data write finish.
+Awaitility.await().atMost(2, TimeUnit.SECONDS).until(
+() -> callbackWithCounter.finishCounter.get() +
callbackWithCounter.failureCounter.get() == writeCount
+);
+assertEquals(callbackWithCounter.failureCounter.get(), 0);
+// Assert metrics stat.
+verifyTheHistogramMetrics(0, 0, 0);
+// cleanup.
+txnLogBufferedWriter.close();
+metricsStats.close();
+transactionTimer.stop();
+orderedExecutor.shutdown();
+}
+
+@Test
+public void testMetricsStatsThatTriggeredByMaxRecordCount() throws
Exception {
+SumStrDataSerializer dataSerializer = new SumStrDataSerializer();
+int batchedWriteMaxRecords = 2;
+int writeCount = 100;
+int expectedBatchFlushCount = writeCount / batchedWriteMaxRecords;
+int expectedTotalBytesSize = writeCount *
dataSerializer.getSizePerData();
+// Create callback with counter.
+var callbackWithCounter = createCallBackWithCounter();
+// Create TxnLogBufferedWriter.
+var txnLogBufferedWriterContext =
createTxnBufferedWriterContextWithMetrics(
+dataSerializer, batchedWriteMaxRecords, Integer.MAX_VALUE,
Integer.MAX_VALUE);
+var txnLogBufferedWriter =
[GitHub] [pulsar] AnonHxy opened a new pull request, #17000: [branch-2.10][cherry-pick] Fix topic dispatch rate limiter not init on broker-level #16084
AnonHxy opened a new pull request, #17000: URL: https://github.com/apache/pulsar/pull/17000 ### Motivation Cherry-pick https://github.com/apache/pulsar/pull/16084 ### Modifications * cherry-pick https://github.com/apache/pulsar/pull/16084 * Also chery-pick https://github.com/apache/pulsar/pull/15377, which is base on ### Verifying this change - [x] Make sure that the change passes the CI checks. ### Documentation Check the box below or label this PR directly. Need to update docs? - [ ] `doc-required` (Your PR needs to update docs and you will update later) - [x] `doc-not-needed` (Please explain why) - [ ] `doc` (Your PR contains doc changes) - [ ] `doc-complete` (Docs have been already added) -- 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-unsubscr...@pulsar.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [pulsar] poorbarcode commented on a diff in pull request #16758: [improve][txn] PIP-160 Metrics stats of Transaction buffered writer
poorbarcode commented on code in PR #16758:
URL: https://github.com/apache/pulsar/pull/16758#discussion_r940552396
##
pulsar-transaction/coordinator/src/main/java/org/apache/pulsar/transaction/coordinator/impl/TxnLogBufferedWriterMetricsStats.java:
##
@@ -0,0 +1,234 @@
+/**
+ * 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.pulsar.transaction.coordinator.impl;
+
+import io.prometheus.client.Collector;
+import io.prometheus.client.CollectorRegistry;
+import io.prometheus.client.Counter;
+import io.prometheus.client.Histogram;
+import java.io.Closeable;
+import java.util.HashMap;
+
+/***
+ * Describes the working status of the {@link TxnLogBufferedWriter}, helps
users tune the thresholds of
+ * {@link TxnLogBufferedWriter} for best performance.
+ * Note-1: When batch feature is turned off, no data is logged at this. In
this scenario，users can see the
+ *{@link org.apache.bookkeeper.mledger.ManagedLedgerMXBean}.
+ * Note-2: Even if enable batch feature, if a single record is too big, it
still directly write to Bookie without batch,
+ *property {@link #pulsarBatchedLogTriggeringCountByForce} can indicate
this case. But this case will not affect
+ *other metrics, because it would obscure the real situation. E.g. there
has two record:
Review Comment:
Good suggestion, I have already changed the code comment
--
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-unsubscr...@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
[GitHub] [pulsar] poorbarcode commented on a diff in pull request #16758: [improve][txn] PIP-160 Metrics stats of Transaction buffered writer
poorbarcode commented on code in PR #16758: URL: https://github.com/apache/pulsar/pull/16758#discussion_r940550668 ## pulsar-transaction/coordinator/src/main/java/org/apache/pulsar/transaction/coordinator/impl/TxnLogBufferedWriter.java: ## @@ -116,6 +116,16 @@ AtomicReferenceFieldUpdater .newUpdater(TxnLogBufferedWriter.class, TxnLogBufferedWriter.State.class, "state"); +/** Metrics. **/ +private final TxnLogBufferedWriterMetricsStats metricsStats; + +public TxnLogBufferedWriter(ManagedLedger managedLedger, OrderedExecutor orderedExecutor, Timer timer, Review Comment: yes, I will remove the if metrics != null statements in the next PRs. -- 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-unsubscr...@pulsar.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [pulsar] lhotari commented on pull request #16968: [fix][broker]Prevent `StackOverFlowException` in SHARED subscription
lhotari commented on PR #16968: URL: https://github.com/apache/pulsar/pull/16968#issuecomment-1208467658 I was in the impression that #16812 would have fixed the StackOverFlowException. @mattisonchao were you able to reproduce the issue with PR #16812 changes? -- 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-unsubscr...@pulsar.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [pulsar] lhotari commented on a diff in pull request #16968: [fix][broker]Prevent `StackOverFlowException` in SHARED subscription
lhotari commented on code in PR #16968: URL: https://github.com/apache/pulsar/pull/16968#discussion_r940535025 ## pulsar-broker/src/main/java/org/apache/pulsar/broker/service/persistent/PersistentDispatcherMultipleConsumers.java: ## @@ -93,7 +93,7 @@ public class PersistentDispatcherMultipleConsumers extends AbstractDispatcherMul protected volatile PositionImpl minReplayedPosition = null; protected boolean shouldRewindBeforeReadingOrReplaying = false; protected final String name; -protected boolean sendInProgress; +protected volatile boolean sendInProgress; Review Comment: was it necessary to make this volatile? -- 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-unsubscr...@pulsar.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [pulsar] lhotari commented on a diff in pull request #16968: [fix][broker]Prevent `StackOverFlowException` in SHARED subscription
lhotari commented on code in PR #16968:
URL: https://github.com/apache/pulsar/pull/16968#discussion_r940533777
##
pulsar-broker/src/main/java/org/apache/pulsar/broker/service/persistent/PersistentDispatcherMultipleConsumers.java:
##
@@ -544,24 +550,25 @@ public final synchronized void
readEntriesComplete(List entries, Object c
if (serviceConfig.isDispatcherDispatchMessagesInSubscriptionThread()) {
// setting sendInProgress here, because sendMessagesToConsumers
will be executed
// in a separate thread, and we want to prevent more reads
Review Comment:
this comment is now outdated since `sendInProgress = true;` was removed. Was
it intentional to remove `sendInProgress = true`?
--
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-unsubscr...@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
[GitHub] [pulsar] poorbarcode opened a new pull request, #16999: [fix][flaky-test]TxnLogBufferedWriterTest.testMainProcess callback mismatch
poorbarcode opened a new pull request, #16999: URL: https://github.com/apache/pulsar/pull/16999 Fixes #16989 Master Issue: #16989 ### Motivation We mocked a BK error, but if an exception occurs, the failure callback will be executed earlier. So the results of the callbacks will be out of order, resulting in a mismatch with the expected value. ### Modifications If we mocked BK errors, sort the callback results. ### Documentation - [ ] `doc-required` (Your PR needs to update docs and you will update later) - [x] `doc-not-needed` (Please explain why) - [ ] `doc` (Your PR contains doc changes) - [ ] `doc-complete` (Docs have been already added) -- 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-unsubscr...@pulsar.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
