[GitHub] [pulsar-dotpulsar] blankensteiner commented on a diff in pull request #151: [Doc] Add links to client feature matrix in README.md
blankensteiner commented on code in PR #151: URL: https://github.com/apache/pulsar-dotpulsar/pull/151#discussion_r1188166955 ## README.md: ## @@ -99,7 +101,9 @@ We use [SemVer](http://semver.org/) for versioning. For the versions available, * **Daniel Blankensteiner** - *Initial work* - [Danske Commodities](https://github.com/danske-commodities) -See also the list of [contributors](https://github.com/apache/pulsar-dotpulsar/contributors) who participated in this project. +Contributions are welcomed and greatly appreciated. See also the list of [contributors](https://github.com/apache/pulsar-dotpulsar/contributors) who participated in this project. + +If your contribution adds Pulsar features for Go clients, you need to update both the [Pulsar docs](https://pulsar.apache.org/docs/client-libraries/) and the [Client Feature Matrix](https://pulsar.apache.org/client-feature-matrix/). See [Contribution Guide](https://pulsar.apache.org/contribute/site-intro/#pages) for more details. Review Comment: Go 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] michaeljmarshall opened a new pull request, #20269: [feat] Add hostname verification config to ClusterData
michaeljmarshall opened a new pull request, #20269: URL: https://github.com/apache/pulsar/pull/20269 PIP: This will be part of an upcoming PIP. Leaving it as a draft for now. ### Motivation It would be helpful to explicitly configure the pulsar client's hostname verification configuration for each remote cluster in the cluster configuration data. ### Modifications * TBD ### Verifying this change This is a trivial change from a configuration perspective. I expect many tests will fail though, so those will also verify the changes. ### Does this pull request potentially affect one of the following parts: - [x] The default values of configurations ### Documentation - [x] `doc-required` ### Matching PR in forked repository PR in forked repository: https://github.com/michaeljmarshall/pulsar/pull/45 -- 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] rdhabalia commented on pull request #19944: [improve][client] Support multi-topic messageId deserialization to ack messages
rdhabalia commented on PR #19944: URL: https://github.com/apache/pulsar/pull/19944#issuecomment-1539437163 @liangyepianzhou @BewareMyPower It seems [this PIP](https://github.com/apache/pulsar/issues/20221) was created before https://github.com/apache/pulsar/issues/20225 . But anyways, I will rename my PIP to 267 to resolve the conflict. -- 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, #20268: [feat] Enable hostname verification by default
michaeljmarshall opened a new pull request, #20268: URL: https://github.com/apache/pulsar/pull/20268 PIP: This will require a PIP. It is a draft for now while I get tests passing. ### Motivation It is recommended to use hostname verification in most use cases for TLS. In order to have more secure defaults, I propose that we enable TLS hostname verification by default. This change will not affect any users that do not have TLS enabled. It will only be a breaking change for users that want to use TLS with hostname verification disabled. ### Modifications * Update all clients to enable hostname verification by default. ### Verifying this change This is a trivial change from a configuration perspective. I expect many tests will fail though, so those will also verify the changes. ### Does this pull request potentially affect one of the following parts: - [x] The default values of configurations ### Documentation - [x] `doc-required` ### Matching PR in forked repository PR in forked repository: https://github.com/michaeljmarshall/pulsar/pull/44 -- 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, #20267: [feat][fn] JavaInstanceStarter --tls_allow_insecure default to false
michaeljmarshall opened a new pull request, #20267: URL: https://github.com/apache/pulsar/pull/20267 PIP: This will be part of a PIP. It is a draft for now. ### Motivation Pulsar's defaults for allowing insecure connections are always `false` except for in this one case in the `JavaInstanceStarter`. A more secure default is to set it to `false`, so I propose we change this default. ### Modifications * Make the default value for `--tls_allow_insecure` in the `JavaInstanceStarter` class `false. ### Verifying this change I will verify that tests pass. ### Does this pull request potentially affect one of the following parts: - [x] The default values of configurations ### Documentation - [x] `doc-not-needed` These docs are generated. ### Matching PR in forked repository PR in forked repository: https://github.com/michaeljmarshall/pulsar/pull/43 -- 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-site] michaeljmarshall commented on a diff in pull request #558: Minor improvements to OIDC docs
michaeljmarshall commented on code in PR #558: URL: https://github.com/apache/pulsar-site/pull/558#discussion_r1188124365 ## docs/security-openid-connect.md: ## @@ -8,6 +8,10 @@ Apache Pulsar supports authenticating clients using [OpenID Connect](https://ope The source code for the OpenID Connect implementation is in the [pulsar-broker-auth-oidc](https://github.com/apache/pulsar/blob/master/pulsar-broker-auth-oidc/) submodule in the Apache Pulsar git repo. +:::note +Pulsar's OpenID Connect integration was introduced in Pulsar 3.0.0. As always, if you encounter any issues, please ask questions on Pulsar channels and open issues in GitHub. Review Comment: > it's applicable for all features/improvements/... If we add this here, we need to add this everywhere My intent is slightly different here than with other features. I agree that with open source projects, it is often implicit that we accept contributions from anyone interested in taking the time to give them. However, in this section, I wanted to convey that this is a new feature and there might be edge cases that are not covered in the docs. In some projects, they represent this by calling a feature "alpha", "beta", or "GA". We don't have those terms. Is there a different way I can word this to express that sentiment? -- 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-site] michaeljmarshall commented on a diff in pull request #558: Minor improvements to OIDC docs
michaeljmarshall commented on code in PR #558: URL: https://github.com/apache/pulsar-site/pull/558#discussion_r1188123061 ## docs/security-openid-connect.md: ## @@ -21,9 +25,9 @@ After authenticating with the Identity Provider, the Pulsar client gets an acces 7. When token validation is successful, the Pulsar Server extracts the `sub` claim from the token (or the configured `openIDRoleClaim`) and uses it as the principal for authorization. 8. When the token expires, the Pulsar Server challenges the client to re-authenticate with the Identity Provider and provide a new access token. If the client fails to re-authenticate, the Pulsar Server closes the connection. -## Enable OpenID Connect Authentication in the Brokers, Proxies, and WebSocket Proxies +## Enable OpenID Connect Authentication in the Broker and Proxy -To configure Pulsar Servers to authenticate clients using OpenID Connect, add the following parameters to the `conf/broker.conf`, the `conf/proxy.conf`, and the `conf/websocket.conf` files. If you use a standalone Pulsar, you need to add these parameters to the `conf/standalone.conf` file: +To configure Pulsar Servers to authenticate clients using OpenID Connect, add the following parameters to the `conf/broker.conf` and the `conf/proxy.conf`. If you use a standalone Pulsar, you need to add these parameters to the `conf/standalone.conf` file: Review Comment: Do we capitalize broker and proxy in this context? I think I am going to replace server with broker and proxy. -- 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 #20242: [fix][client] Java Client's Seek Logic Not Threadsafe #1
michaeljmarshall commented on code in PR #20242: URL: https://github.com/apache/pulsar/pull/20242#discussion_r1188120436 ## pulsar-client/src/main/java/org/apache/pulsar/client/impl/ConsumerImpl.java: ## @@ -2153,9 +2153,14 @@ private CompletableFuture seekAsyncInternal(long requestId, ByteBuf seek, MessageIdAdv originSeekMessageId = seekMessageId; seekMessageId = (MessageIdAdv) seekId; -duringSeek.set(true); + +if (!duringSeek.compareAndSet(false, true)) { +log.warn("[{}][{}] Attempting to seek operation that is already in progress, cancelling {}", +topic, subscription, seekBy); +seekFuture.cancel(true); Review Comment: Nit: I think an `IllegalStateException` would be a clearer exception indicating to the user that it was not a valid time to call this method. One problem with `CancellationException` is that it has no message. However, I see that there is a warning log indicating the problem, so it might be fine. -- 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] lvs071103 edited a discussion: how to delete ledger
GitHub user lvs071103 edited a discussion: how to delete ledger Because the disk space is full, bookkeeper reports the following error, how to delete ledger, or do you have any recommendations ``` 12:04:13.283 [BookKeeperClientWorker-OrderedExecutor-3-0] ERROR org.apache.bookkeeper.proto.PerChannelBookieClient - Read for failed on bookie 10.1.69.169:3181 code EIO 12:04:13.283 [BookKeeperClientWorker-OrderedExecutor-3-0] INFO org.apache.bookkeeper.client.PendingReadOp - Error: Error while reading ledger while reading L6903 E7473 from bookie: 10.1.69.169:3181 12:04:13.306 [BookKeeperClientWorker-OrderedExecutor-3-0] ERROR org.apache.bookkeeper.proto.PerChannelBookieClient - Read for failed on bookie 10.1.69.169:3181 code EIO 12:04:13.306 [BookKeeperClientWorker-OrderedExecutor-3-0] INFO org.apache.bookkeeper.client.PendingReadOp - Error: Error while reading ledger while reading L6903 E7475 from bookie: 10.1.69.169:3181 ``` GitHub link: https://github.com/apache/pulsar/discussions/20266 This is an automatically sent email for commits@pulsar.apache.org. To unsubscribe, please send an email to: commits-unsubscr...@pulsar.apache.org
[GitHub] [pulsar] michaeljmarshall commented on pull request #20151: [fix][broker] Fix auth test cases.
michaeljmarshall commented on PR #20151: URL: https://github.com/apache/pulsar/pull/20151#issuecomment-1539375090 > Hi, @michaeljmarshall After revert #20068, do you have any suggestion to fix the test cases? Maybe be fixed by #20145. Are the modified tests failing in master? -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: commits-unsubscr...@pulsar.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [pulsar-client-python] Samreay commented on issue #55: Python3 AsyncIO implementation
Samreay commented on issue #55: URL: https://github.com/apache/pulsar-client-python/issues/55#issuecomment-1539374455 I think a larger point to possibly be made here is the difficulty of using pulsar with other libraries without these features. For example, consider a user trying to write a simple FastAPI server that simply exposes a REST endpoint for a client that gives that latest value in a topic. If they were using Kafka, they could use `broadcaster` (like suggested [here](https://github.com/tiangolo/fastapi/discussions/6918)), or they could use `aiokafka` (an example of this would be [here](https://github.com/pedrodeoliveira/fastapi-kafka-consumer) and [here](https://github.com/tiangolo/fastapi/discussions/6992)). These third party libraries that allow background processing generally require coroutines to be passed in and don't support synchronous functions because that's obviously a nightmare with python and GIL. I'm not even sure on how to nicely implement the FastAPI example with Pulsar client, or if the increased complexity means the realistic solution is "Yeah just use kafka" -- 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 added a comment to the discussion: Build distroless package for better security, smaller size, speed and more
GitHub user michaeljmarshall added a comment to the discussion: Build distroless package for better security, smaller size, speed and more This would be a great feature to add! GitHub link: https://github.com/apache/pulsar/discussions/20253#discussioncomment-5844529 This is an automatically sent email for commits@pulsar.apache.org. To unsubscribe, please send an email to: commits-unsubscr...@pulsar.apache.org
[GitHub] [pulsar] michaeljmarshall added a comment to the discussion: Build distroless package for better security, smaller size, speed and more
GitHub user michaeljmarshall added a comment to the discussion: Build distroless package for better security, smaller size, speed and more Also, pulsar's configuration could be ready for an overhaul. There is an ongoing discussion about configuration here https://lists.apache.org/thread/ok6hnvwsfcckj50471tkfbbhtgo6ng35 GitHub link: https://github.com/apache/pulsar/discussions/20253#discussioncomment-5844525 This is an automatically sent email for commits@pulsar.apache.org. To unsubscribe, please send an email to: commits-unsubscr...@pulsar.apache.org
[GitHub] [pulsar] michaeljmarshall added a comment to the discussion: Build distroless package for better security, smaller size, speed and more
GitHub user michaeljmarshall added a comment to the discussion: Build distroless package for better security, smaller size, speed and more > => maybe this is a high value long term topic for Pulsar 4.0 Based on our current versioning scheme, this kind of change could be introduced in a minor release, like 3.1. GitHub link: https://github.com/apache/pulsar/discussions/20253#discussioncomment-5844519 This is an automatically sent email for commits@pulsar.apache.org. To unsubscribe, please send an email to: commits-unsubscr...@pulsar.apache.org
[GitHub] [pulsar] michaeljmarshall commented on pull request #20230: [fix][monitor] topic with double quote breaks the prometheus format
michaeljmarshall commented on PR #20230: URL: https://github.com/apache/pulsar/pull/20230#issuecomment-1539344842 /pulsarbot rerun-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-node] XXXMrG commented on a diff in pull request #322: [feat]: Support client.getPartitionsForTopic (#320)
XXXMrG commented on code in PR #322: URL: https://github.com/apache/pulsar-client-node/pull/322#discussion_r1188071929 ## tests/client.test.js: ## @@ -49,5 +74,54 @@ const Pulsar = require('../index.js'); })).toThrow('Service URL is required and must be specified as a string'); }); }); +describe('test getPartitionsForTopic', () => { + test('GetPartitions for empty topic', async () => { +const client = new Pulsar.Client({ + serviceUrl: 'pulsar://localhost:6650', + operationTimeoutSeconds: 30, +}); + +await expect(client.getPartitionsForTopic('')) + .rejects.toThrow('Failed to GetPartitionsForTopic: InvalidTopicName'); +await client.close(); + }); + + test('Client/getPartitionsForTopic', async () => { +const client = new Pulsar.Client({ + serviceUrl: 'pulsar://localhost:6650', + operationTimeoutSeconds: 30, +}); + +// test on nonPartitionedTopic +const nonPartitionedTopicName = 'test-non-partitioned-topic'; +const nonPartitionedTopic = `persistent://public/default/${nonPartitionedTopicName}`; +await requestAdminApi(`${baseUrl}/admin/v2/persistent/public/default/${nonPartitionedTopicName}`, { + headers: { +'Content-Type': 'application/json', + }, +}); +const nonPartitionedTopicList = await client.getPartitionsForTopic(nonPartitionedTopic); Review Comment: Got it, let me try again. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: commits-unsubscr...@pulsar.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [pulsar] rdhabalia opened a new issue, #20265: PIP-267: Add support of topic stats/stats-internal client api
rdhabalia opened a new issue, #20265: URL: https://github.com/apache/pulsar/issues/20265 # Motivation Right now, Pulsar provides the topic's stats and stats-internal over HTTP admin API, and this stats data is used by user applications and also by Pulsar internal components such as Pulsar-functions to derive the certain states of the applications. for example, there are use cases where the application wants to check the topic's backlog, subscription's state (readPosition, list of subscriptions), numberOfEntriesSinceFirstNotAckedMessage, etc to bootstrap the application or handle the application’s resiliency and state dynamically. Applications can retrieve this stats information by using the broker’s admin HTTP APIs. However, stats retrieval over HTTP API doesn’t work well in use cases when users would like to access this API at a higher scale when a large number of application nodes would like to use it over HTTP which could overload brokers and sometimes makes broker irresponsive and impact admin API performance. It’s also become difficult when Pulsar is deployed in the cloud behind the SNI proxy and applications also want to access large-scale stats information periodically over different HTTP port instead it would be better if applications can fetch stats over on same binary protocol for scalability and accessibility reasons. Therefore, there are multiple use cases where producer/consumer applications need stats information for topics using the client library over binary protocol. Hence, this PIP introduces client API for producers and consumers to access topic stats/internal-stats information which can be used by applications as needed. # Client library changes The client library will allow both producer and consumer to retrieve stats using `TopicStatsProvider` that fetches stats/internal-stats for both persistent and non-persistent topics. Producer/consumer of a partition can use the same connection with the broker to access topic and retrieve topics stats. Pulsar also allows application to access multiple different partitions/topics using single producer/consumer so, that producer/consumer should be able to provide generic API to access stats of single or multi topics/partitions served by them. Therefore, producer/consumer will have additional API that returns `TopicStatsProvider` which allows the application to fetch stats/internal-stats of all topics/partitions served by that producer/consumer. ``` Producer.java / Consumer.java /** * Returns {@link TopicStatsProvider} to retrieve stats/internal-stats of the topic. */ TopicStatsProvider getTopicStatsProvider(); ``` ``` TopicStatsProvider.java /** * TopicStatsProvider provides API to access topic’s stats and internal-stat information. */ public interface TopicStatsProvider { /** * @return the topic stats */ CompletableFuture getStats(); /** * @return the internal topic stats */ CompletableFuture getInternalStats(); } ``` ``` TopicStatsInfo.java /** * TopicStatsInfo contains the stats information of all partitions of a given topic. It allows the client to access stats of each individual partition from the partition stats map. * public class TopicStatsInfo { private Map partitions; } TopicInternalStatsInfo.java /** * TopicInternalStatsInfo contains internal-stats information of topic. It allows the client to access stats of partitioned and non-partitioned topics. */ public class TopicInternalStatsInfo { private Map partitions; } ``` We would like to create create a generic stats protocol between client and broker so, broker can send any type of stats (eg: stats or internal-stats) by serializing into specific format and client can deserialize it into appropriate format and return back to the client application. Therefore, this PIP will introduce new wire-protocol for `TopicStats` where client sends stats-type and topic-partition to the broker and broker sends back serialized stats response which can be deserialized by client based on stats type format. ``` message CommandTopicStats { enum StatsType { STATS = 0; STATS_INTERNAL = 1; } required uint64 request_id= 1; required string topic_name= 2; required StatsType stats_type = 3; } message CommandTopicStatsResponse { required uint64 request_id= 1; optional ServerError error_code= 2; optional string error_message = 3; optional string stats_json= 4; } ``` # Broker changes A broker will have support to handle the stats command for a partition and return stats/internal-stats response for a partition. Broker will authorize connected client if client is authorized to
[GitHub] [pulsar] equanz commented on pull request #20179: [fix][broker] Fix recentlyJoinedConsumers to address the out-of-order issue
equanz commented on PR #20179: URL: https://github.com/apache/pulsar/pull/20179#issuecomment-1539314184 @poorbarcode * [Q-1] Yes. * [Q-2] When I created this PR, I introduced Map structure because it was necessary. For more details, please see the description("2. Keep the last sent position per key."). I just noticed we could avoid introducing a Map structure if we implemented the last sent position feature, like the mark delete position and the individually deleted messages feature. In other words, - The position is already scheduled to be sent. - All positions less than or equal to it are already scheduled to be sent. - Manage individually sent positions to update the position as expected. Consider the following flow. 1. Assume that the [entries](https://github.com/apache/pulsar/blob/35e9897742b7db4bd29349940075a819b2ad6999/pulsar-broker/src/main/java/org/apache/pulsar/broker/service/persistent/PersistentStickyKeyDispatcherMultipleConsumers.java#L174) has the following messages, - `msg-1`, key: `key-a`, position: `1:1` - `msg-2`, key: `key-a`, position: `1:2` - `msg-3`, key: `key-a`, position: `1:3` - `msg-4`, key: `key-b`, position: `1:4` - `msg-5`, key: `key-b`, position: `1:5` - `msg-6`, key: `key-b`, position: `1:6` the dispatcher has two consumers (`c1` `messagesForC` is 1, `c2` `messageForC` is 1000), and the selector will return `c1` if `key-a` and `c2` if `key-b`. 2. Send `msg-1` to `c1` and `msg-4` - `msg-6` to `c2`. - So, the current last sent position is `1:1` and the individually sent positions is `[[1:3, 1:6]]` (list of closed intervals. not list of list). - `c1` never acknowledge `msg-1`. **scenario A** Add new consumer c3, and the selector will return c3 if key-a. Can't send msg-2 - msg-3 to c3 because 1:2, and 1:3 are greater than the last sent position, 1:1. Disconnect c1. Send msg-1 - msg-3 to c3.Now, c3 receives messages with expected order about key-a. **scenario B** c1 messagesForC is back to 999. Send msg-2 - msg-3 to c1 So, the current last sent position is 1:6, and the individually sent positions is []. My motivation is not to improve Key_Shared but fix Key_Shared. Therefore, if the current approach is not accepted as a fix, I introduce it and remove the feature by Map per key. -- 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 #20261: [fix][broker] Fix `UnsupportedOperationException` when update topic properties.
poorbarcode commented on code in PR #20261: URL: https://github.com/apache/pulsar/pull/20261#discussion_r1188063931 ## pulsar-broker/src/test/java/org/apache/pulsar/broker/admin/TopicPoliciesTest.java: ## @@ -133,6 +133,20 @@ public void cleanup() throws Exception { super.internalCleanup(); } +@Test +public void updatePropertiesForAutoCreatedTopicTest() throws Exception { +TopicName topicName = TopicName.get( +TopicDomain.persistent.value(), +NamespaceName.get(myNamespace), +"test-" + UUID.randomUUID() +); +String testTopic = topicName.toString(); +Producer producer = pulsarClient.newProducer().topic(testTopic).create(); Review Comment: +1 And suggestion to remove this topic. ## pulsar-broker/src/main/java/org/apache/pulsar/broker/admin/impl/PersistentTopicsBase.java: ## @@ -665,7 +665,7 @@ protected CompletableFuture internalUpdatePropertiesAsync(boolean authorit return validateTopicOwnershipAsync(topicName, authoritative) .thenCompose(__ -> validateTopicOperationAsync(topicName, TopicOperation.UPDATE_METADATA)) .thenCompose(__ -> { -if (topicName.isPartitioned()) { +if (!topicName.isPartitioned()) { Review Comment: +1 -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: commits-unsubscr...@pulsar.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [pulsar-client-node] shibd commented on a diff in pull request #322: [feat]: Support client.getPartitionsForTopic (#320)
shibd commented on code in PR #322: URL: https://github.com/apache/pulsar-client-node/pull/322#discussion_r1188061390 ## tests/client.test.js: ## @@ -49,5 +74,54 @@ const Pulsar = require('../index.js'); })).toThrow('Service URL is required and must be specified as a string'); }); }); +describe('test getPartitionsForTopic', () => { + test('GetPartitions for empty topic', async () => { +const client = new Pulsar.Client({ + serviceUrl: 'pulsar://localhost:6650', + operationTimeoutSeconds: 30, +}); + +await expect(client.getPartitionsForTopic('')) + .rejects.toThrow('Failed to GetPartitionsForTopic: InvalidTopicName'); +await client.close(); + }); + + test('Client/getPartitionsForTopic', async () => { +const client = new Pulsar.Client({ + serviceUrl: 'pulsar://localhost:6650', + operationTimeoutSeconds: 30, +}); + +// test on nonPartitionedTopic +const nonPartitionedTopicName = 'test-non-partitioned-topic'; +const nonPartitionedTopic = `persistent://public/default/${nonPartitionedTopicName}`; +await requestAdminApi(`${baseUrl}/admin/v2/persistent/public/default/${nonPartitionedTopicName}`, { + headers: { +'Content-Type': 'application/json', + }, +}); +const nonPartitionedTopicList = await client.getPartitionsForTopic(nonPartitionedTopic); Review Comment: Before this, you should assert the HTTP response code first. -- 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-node] shibd commented on pull request #322: [feat]: Support client.getPartitionsForTopic (#320)
shibd commented on PR #322: URL: https://github.com/apache/pulsar-client-node/pull/322#issuecomment-1539309252 ``` 2023-05-09T02:41:35,995+ [pulsar-web-48-8] WARN org.eclipse.jetty.server.HttpChannelState - unhandled due to prior sendError javax.servlet.ServletException: javax.servlet.ServletException: java.lang.NumberFormatException: For input string: ""4"" at org.eclipse.jetty.server.handler.HandlerCollection.handle(HandlerCollection.java:162) ~[org.eclipse.jetty-jetty-server-9.4.48.v20220622.jar:9.4.48.v20220622] at org.eclipse.jetty.server.handler.StatisticsHandler.handle(StatisticsHandler.java:181) ~[org.eclipse.jetty-jetty-server-9.4.48.v20220622.jar:9.4.48.v20220622] ``` Maybe the request body format is invalid. Will cause the partition topic to create failed. 1. You should assert the HTTP response code. 2. You should be change request body format: ```diff - data: '4', + data: 4, ``` -- 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- closed pull request #20264: [improve][build] Upgrade dependency to fix CVE.
Technoboy- closed pull request #20264: [improve][build] Upgrade dependency to fix CVE. URL: https://github.com/apache/pulsar/pull/20264 -- 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, #20264: [improve][build] Upgrade dependency to fix CVE.
Technoboy- opened a new pull request, #20264: URL: https://github.com/apache/pulsar/pull/20264 ### Motivation Upgrade dependency to fix CVE. ### Documentation - [ ] `doc` - [ ] `doc-required` - [x] `doc-not-needed` - [ ] `doc-complete` -- 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- merged pull request #20254: [improve][ci][branch-3.0] Enable CI tests for PRs
Technoboy- merged PR #20254: URL: https://github.com/apache/pulsar/pull/20254 -- 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-3.0 updated: [improve][ci] Enable CI tests for PRs (#20254)
This is an automated email from the ASF dual-hosted git repository. technoboy pushed a commit to branch branch-3.0 in repository https://gitbox.apache.org/repos/asf/pulsar.git The following commit(s) were added to refs/heads/branch-3.0 by this push: new bfe08a9a4ac [improve][ci] Enable CI tests for PRs (#20254) bfe08a9a4ac is described below commit bfe08a9a4ac893a96cadfa51567f6b696e021324 Author: Zike Yang AuthorDate: Tue May 9 10:06:36 2023 +0800 [improve][ci] Enable CI tests for PRs (#20254) --- .github/workflows/ci-go-functions.yaml | 1 + .github/workflows/pulsar-ci-flaky.yaml | 1 + .github/workflows/pulsar-ci.yaml | 1 + 3 files changed, 3 insertions(+) diff --git a/.github/workflows/ci-go-functions.yaml b/.github/workflows/ci-go-functions.yaml index a34cb1a9090..1ff0c8eef57 100644 --- a/.github/workflows/ci-go-functions.yaml +++ b/.github/workflows/ci-go-functions.yaml @@ -22,6 +22,7 @@ on: pull_request: branches: - master + - branch-3.0 paths: - '.github/workflows/**' - 'pulsar-function-go/**' diff --git a/.github/workflows/pulsar-ci-flaky.yaml b/.github/workflows/pulsar-ci-flaky.yaml index ea19bc33078..198823c3008 100644 --- a/.github/workflows/pulsar-ci-flaky.yaml +++ b/.github/workflows/pulsar-ci-flaky.yaml @@ -22,6 +22,7 @@ on: pull_request: branches: - master + - branch-3.0 schedule: - cron: '0 12 * * *' workflow_dispatch: diff --git a/.github/workflows/pulsar-ci.yaml b/.github/workflows/pulsar-ci.yaml index 4d4f2902d8f..3e2a67e3cf0 100644 --- a/.github/workflows/pulsar-ci.yaml +++ b/.github/workflows/pulsar-ci.yaml @@ -22,6 +22,7 @@ on: pull_request: branches: - master + - branch-3.0 schedule: - cron: '0 12 * * *' workflow_dispatch:
[GitHub] [pulsar] Technoboy- commented on a diff in pull request #20261: [fix][broker] Fix `UnsupportedOperationException` when update topic properties.
Technoboy- commented on code in PR #20261: URL: https://github.com/apache/pulsar/pull/20261#discussion_r1188040910 ## pulsar-broker/src/main/java/org/apache/pulsar/broker/admin/impl/PersistentTopicsBase.java: ## @@ -665,7 +665,7 @@ protected CompletableFuture internalUpdatePropertiesAsync(boolean authorit return validateTopicOwnershipAsync(topicName, authoritative) .thenCompose(__ -> validateTopicOperationAsync(topicName, TopicOperation.UPDATE_METADATA)) .thenCompose(__ -> { -if (topicName.isPartitioned()) { +if (!topicName.isPartitioned()) { Review Comment: Why change 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] gaoran10 commented on a diff in pull request #20261: [fix][broker] Fix `UnsupportedOperationException` when update topic properties.
gaoran10 commented on code in PR #20261: URL: https://github.com/apache/pulsar/pull/20261#discussion_r1188038590 ## pulsar-broker/src/test/java/org/apache/pulsar/broker/admin/TopicPoliciesTest.java: ## @@ -133,6 +133,20 @@ public void cleanup() throws Exception { super.internalCleanup(); } +@Test +public void updatePropertiesForAutoCreatedTopicTest() throws Exception { +TopicName topicName = TopicName.get( +TopicDomain.persistent.value(), +NamespaceName.get(myNamespace), +"test-" + UUID.randomUUID() +); +String testTopic = topicName.toString(); +Producer producer = pulsarClient.newProducer().topic(testTopic).create(); Review Comment: Please close the producer. -- 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- closed pull request #20250: [fix][broker] Fix default bundle size used while setting bookie affinity
Technoboy- closed pull request #20250: [fix][broker] Fix default bundle size used while setting bookie affinity URL: https://github.com/apache/pulsar/pull/20250 -- 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] Samreay edited a discussion: [Python Client] How to subscribe to two topics - both with different schemas?
GitHub user Samreay edited a discussion: [Python Client] How to subscribe to two topics - both with different schemas? Hi all, I'm trying to write up a simple proof-of-concept application, wherein all it does it subscribe to two separate topics, each with their own different AvroSchema. So far, this doesn't seem possible to do easily with the (synchronous) python client, because each `Consumer` only supports one type of schema, and a `consumer.receive()` blocks. Does anyone know a nice solution for this? GitHub link: https://github.com/apache/pulsar/discussions/20263 This is an automatically sent email for commits@pulsar.apache.org. To unsubscribe, please send an email to: commits-unsubscr...@pulsar.apache.org
[GitHub] [pulsar-client-node] shibd opened a new pull request, #324: Upgrade unit test depends on the pulsar version to the latest(currently 3.0.0)
shibd opened a new pull request, #324: URL: https://github.com/apache/pulsar-client-node/pull/324 ### Motivation Using the latest pulsar version to run unit tests helps us develop new features and test compatibility on time. ### Modifications - Upgrade unit test depends on the pulsar version to the latest(currently 3.0.0). ### Verifying this change - All unit test pass means ok. ### 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
[GitHub] [pulsar] tisonkun commented on a diff in pull request #20259: [fix][io] add protobuf ByteString to pulsar-io jdbc core
tisonkun commented on code in PR #20259: URL: https://github.com/apache/pulsar/pull/20259#discussion_r1188018359 ## pulsar-io/jdbc/core/pom.xml: ## @@ -58,6 +58,13 @@ com.fasterxml.jackson.dataformat jackson-dataformat-yaml + + + com.google.protobuf + protobuf-java + 3.22.4 Review Comment: @bpereto Also, please verify that `provided` statisfy your use case. One downside of `provided` is that - I'm not sure - if we don't pack protobuf into the NARball, it's possible to meet `NoClassDefFoundError`. -- 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 a diff in pull request #20259: [fix][io] add protobuf ByteString to pulsar-io jdbc core
tisonkun commented on code in PR #20259: URL: https://github.com/apache/pulsar/pull/20259#discussion_r1188017703 ## pulsar-io/jdbc/core/pom.xml: ## @@ -58,6 +58,13 @@ com.fasterxml.jackson.dataformat jackson-dataformat-yaml + + + com.google.protobuf + protobuf-java + 3.22.4 Review Comment: @bpereto Just remove the `` line. We already import: ```xml com.google.protobuf protobuf-bom ${protobuf3.version} pom import ``` -- 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-site] Anonymitaet commented on a diff in pull request #562: Add PIP-195 to 3.0 release note
Anonymitaet commented on code in PR #562: URL: https://github.com/apache/pulsar-site/pull/562#discussion_r1188004011 ## release-notes/versioned/pulsar-3.0.0.md: ## @@ -68,49 +69,59 @@ sidebar_label: Apache Pulsar 3.0.0 - Implement load data store by @Demogorgon314 in [#18777](https://github.com/apache/pulsar/pull/18777) - Implement broker registry for new load manager by @Demogorgon314 in [#18810](https://github.com/apache/pulsar/pull/18810) - Added ServiceUnitStateChannelImpl by @heesung-sn in [#18489](https://github.com/apache/pulsar/pull/18489) +- [Broker] PIP-195: New bucket based delayed message tracker [#16763](https://github.com/apache/pulsar/issues/16763) + - New bucket based delayed message tracker - interface -part 1 by @coderzc in [#17344](https://github.com/apache/pulsar/pull/17344) + - Implement delayed message index bucket snapshot (create/load) - part2 by @coderzc in [#17611](https://github.com/apache/pulsar/pull/17611) + - Implement BookkeeperBucketSnapshotStorage - part3 by @coderzc in [#17677](https://github.com/apache/pulsar/pull/17677) + - Support internal cursor properties - part4 by @coderzc in [#17712](https://github.com/apache/pulsar/pull/17712) + - Implement BucketDelayedDeliveryTrackerFactory and load BucketDelayedDeliveryTracker - part6 by @coderzc in [#17756](https://github.com/apache/pulsar/pull/17756) + - Implement delayed message bucket snapshot recover - part5 by @coderzc in [#18420](https://github.com/apache/pulsar/pull/18420) + - Implement Filter out all delayed messages and skip them when reading messages from bookies - part7 by @coderzc in [#19035](https://github.com/apache/pulsar/pull/19035) + - Fix cursor skip read by @coderzc in [#19124](https://github.com/apache/pulsar/pull/19124) + - Implement delayed message index bucket snapshot(merge/delete) - part8 by @coderzc in [#19138](https://github.com/apache/pulsar/pull/19138) + - Make BucketDelayedDeliveryTracker can retry snapshot operation & improve logs by @coderzc in [#19577](https://github.com/apache/pulsar/pull/19577) + - Fix BucketDelayedDeliveryTracker merge issues by @coderzc in [#19615](https://github.com/apache/pulsar/pull/19615) + - Cut off snapshot segment according to maxIndexesPerBucketSnapshotSegment by @coderzc in [#19706](https://github.com/apache/pulsar/pull/19706) + - Add metrics for bucket delayed message tracker by @coderzc in [#19716](https://github.com/apache/pulsar/pull/19716) + - Don't clean up BucketDelayedDeliveryTracker when all consumer disconnect by @coderzc in [#19801](https://github.com/apache/pulsar/pull/19801) + - Add topicName and cursorName for ledger metadata of bucket snapshot by @coderzc in [#19802](https://github.com/apache/pulsar/pull/19802) + - Make bucket merge operation asynchronous by @coderzc in [#19873](https://github.com/apache/pulsar/pull/19873) + - Clear delayed message when unsubscribe & Make clear operation asynchronous by @coderzc in [#19901](https://github.com/apache/pulsar/pull/19901) + - Merge multiple buckets at once by @coderzc in [#19927](https://github.com/apache/pulsar/pull/19927) + - Ensure previous delayed index be removed from snapshotSegmentLastIndexTable & Make load operate asynchronous by @coderzc in [#20086](https://github.com/apache/pulsar/pull/20086) + - Fix avoid future of clear delayed message can't complete by @coderzc in [#20075](https://github.com/apache/pulsar/pull/20075) Review Comment: ```suggestion - Fix the issue of delayed messages can't be completed by @coderzc in [#20075](https://github.com/apache/pulsar/pull/20075) ``` Do you mean 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] bpereto commented on a diff in pull request #20259: [fix][io] add protobuf ByteString to pulsar-io jdbc core
bpereto commented on code in PR #20259: URL: https://github.com/apache/pulsar/pull/20259#discussion_r1187891364 ## pulsar-io/jdbc/core/pom.xml: ## @@ -58,6 +58,13 @@ com.fasterxml.jackson.dataformat jackson-dataformat-yaml + + + com.google.protobuf + protobuf-java + 3.22.4 Review Comment: I set the version to `${protobuf3.version}` from `pulsar/pom.xml` and added the provided scope. Is this what you ment by `dependencyManagement` ? -- 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] entvex commented on issue #19877: [Doc] Add API doc for the C# client
entvex commented on issue #19877: URL: https://github.com/apache/pulsar/issues/19877#issuecomment-1538966649 Awesome @tisonkun ! -- 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: [feat][ws] Use async auth method to support OIDC (#20238)
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 03dc3db0a63 [feat][ws] Use async auth method to support OIDC (#20238) 03dc3db0a63 is described below commit 03dc3db0a6357dcde294e55c1933bd13275f3f73 Author: Michael Marshall AuthorDate: Mon May 8 14:15:32 2023 -0500 [feat][ws] Use async auth method to support OIDC (#20238) Fixes #20236 PIP: #19409 ### Motivation In the `AuthenticationService`, we are currently using the deprecated `authenticate` methods. As a result, we hit the `Not Implemented` exception when using the `AuthenticationProviderOpenID`. This PR updates the implementation so that we're able This solution isn't ideal for two reasons. 1. We are not using the `authenticationHttpRequest` method, which seems like the right method for the WebSocket proxy. However, this is not a viable option, as I documented in #20237. 2. We are calling `.get()` on a future. However, it is expected that the `AuthenticationProvider` not block forever, so I think this is acceptable for now. Please let me know if you disagree. ### Modifications * Replace `authenticate` with `authenticateAsync`. ### Verifying this change This change is a trivial rework / code cleanup without any test coverage. ### Documentation - [x] `doc-not-needed` Note that I do have documentation showing that 3.0.x does not support OIDC in the WebSocket Proxy. The `next` docs don't need that limitation since this PR fixes that and targets 3.1.0. https://github.com/apache/pulsar-site/pull/558 ### Matching PR in forked repository PR in forked repository: skipping for this trivial PR --- .../pulsar/broker/authentication/AuthenticationService.java | 13 ++--- 1 file changed, 10 insertions(+), 3 deletions(-) diff --git a/pulsar-broker-common/src/main/java/org/apache/pulsar/broker/authentication/AuthenticationService.java b/pulsar-broker-common/src/main/java/org/apache/pulsar/broker/authentication/AuthenticationService.java index d11bb6d76e8..22296b86b4e 100644 --- a/pulsar-broker-common/src/main/java/org/apache/pulsar/broker/authentication/AuthenticationService.java +++ b/pulsar-broker-common/src/main/java/org/apache/pulsar/broker/authentication/AuthenticationService.java @@ -27,6 +27,7 @@ import java.util.HashMap; import java.util.List; import java.util.Map; import java.util.Optional; +import java.util.concurrent.ExecutionException; import java.util.stream.Collectors; import javax.naming.AuthenticationException; import javax.servlet.http.HttpServletRequest; @@ -171,20 +172,26 @@ public class AuthenticationService implements Closeable { authData = authenticationState.getAuthDataSource(); } // Backward compatible, the authData value was null in the previous implementation -return providerToUse.authenticate(authData); +return providerToUse.authenticateAsync(authData).get(); } catch (AuthenticationException e) { if (LOG.isDebugEnabled()) { LOG.debug("Authentication failed for provider " + providerToUse.getAuthMethodName() + " : " + e.getMessage(), e); } throw e; +} catch (ExecutionException | InterruptedException e) { +if (LOG.isDebugEnabled()) { +LOG.debug("Authentication failed for provider " + providerToUse.getAuthMethodName() + " : " ++ e.getMessage(), e); +} +throw new RuntimeException(e); } } else { for (AuthenticationProvider provider : providers.values()) { try { AuthenticationState authenticationState = provider.newHttpAuthState(request); -return provider.authenticate(authenticationState.getAuthDataSource()); -} catch (AuthenticationException e) { +return provider.authenticateAsync(authenticationState.getAuthDataSource()).get(); +} catch (ExecutionException | InterruptedException | AuthenticationException e) { if (LOG.isDebugEnabled()) { LOG.debug("Authentication failed for provider " + provider.getAuthMethodName() + ": " + e.getMessage(), e);
[GitHub] [pulsar] michaeljmarshall closed issue #20236: Add support for OpenID Connect in WebSocket Proxy
michaeljmarshall closed issue #20236: Add support for OpenID Connect in WebSocket Proxy URL: https://github.com/apache/pulsar/issues/20236 -- 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 merged pull request #20238: [feat][ws] Use async auth method to support OIDC
michaeljmarshall merged PR #20238: URL: https://github.com/apache/pulsar/pull/20238 -- 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] crossoverJie opened a new issue, #20262: [Bug] When using the admin.topics().skipMessages API, the actual number of skipped messages may be less than the expected number when the val
crossoverJie opened a new issue, #20262: URL: https://github.com/apache/pulsar/issues/20262 ### Search before asking - [X] I searched in the [issues](https://github.com/apache/pulsar/issues) and found nothing similar. ### Version latest version ### Minimal reproduce step ```java @Test(dataProvider = "topicName") public void testSkipMessages(String topicName) throws Exception { final String subName = topicName; assertEquals(admin.topics().getList("prop-xyz/ns1"), new ArrayList<>()); final String persistentTopicName = "persistent://prop-xyz/ns1/" + topicName; // Force to create a topic publishMessagesOnPersistentTopic("persistent://prop-xyz/ns1/" + topicName, 0); assertEquals(admin.topics().getList("prop-xyz/ns1"), List.of("persistent://prop-xyz/ns1/" + topicName)); // create consumer and subscription @Cleanup PulsarClient client = PulsarClient.builder() .serviceUrl(pulsar.getWebServiceAddress()) .statsInterval(0, TimeUnit.SECONDS) .build(); AtomicInteger total = new AtomicInteger(); Consumer consumer = client.newConsumer().topic(persistentTopicName) .messageListener(new MessageListener() { @SneakyThrows @Override public void received(Consumer consumer, Message msg) { if (total.get() %2 !=0){ log.info("msg_id{}",msg.getMessageId().toString()); consumer.acknowledge(msg); } total.incrementAndGet(); } }) .subscriptionName(subName) .subscriptionType(SubscriptionType.Exclusive).subscribe(); assertEquals(admin.topics().getSubscriptions(persistentTopicName), List.of(subName)); publishMessagesOnPersistentTopic("persistent://prop-xyz/ns1/" + topicName, 100); TimeUnit.SECONDS.sleep(3); TopicStats topicStats = admin.topics().getStats(persistentTopicName); long msgBacklog = topicStats.getSubscriptions().get(subName).getMsgBacklog(); log.info("back={}",msgBacklog); int skipNumber = 20; admin.topics().skipMessages(persistentTopicName, subName, skipNumber); topicStats = admin.topics().getStats(persistentTopicName); assertEquals(topicStats.getSubscriptions().get(subName).getMsgBacklog(), msgBacklog - skipNumber); } ``` **When I artificially created 50 hollow messages** and skipped 20 messages, only 14 messages were actually skipped. ### What did you expect to see? The expected number of skipped messages is equal to the actual number of skipped messages. ### What did you see instead? The actual number of skips is less than the expected number of skips. https://user-images.githubusercontent.com/15684156/236889435-cfb6098b-eb90-42b2-bce1-fc3273fc583e.png;> ### Anything else? https://github.com/apache/pulsar/blob/e956db729f5098ff319237fd4220dbfb234c1b18/managed-ledger/src/main/java/org/apache/bookkeeper/mledger/impl/ManagedCursorImpl.java#L1772-L1785 https://user-images.githubusercontent.com/15684156/236890402-dd9a5023-e597-432f-b188-abd6443a3965.png;> The reason for this issue is that the `recycle()` function reuses objects, causing the object referenced by r to change during runtime. When the loop count is greater than 8, the else block is entered, leading to an incorrect calculation of the message count. I believe there are two methods to fix this issue. The first method is to perform a copy before assignment, similar to the following: ```java public void setStartPosition(PositionImpl startPosition) { PositionImpl cp = new PositionImpl(startPosition.ledgerId, startPosition.entryId); this.startPosition = cp; } public void setEndPosition(PositionImpl endPosition) { PositionImpl cp = new PositionImpl(endPosition.ledgerId, endPosition.entryId); this.endPosition = cp; } // state.endPosition = r.lowerEndpoint(); state.setEndPosition(r.lowerEndpoint()); // state.startPosition = r.upperEndpoint(); state.setStartPosition(r.upperEndpoint()); ``` The second method is to remove `recyclePositionRangeConverter`. https://user-images.githubusercontent.com/15684156/236891897-138ec453-a2df-4e4b-862d-eb65a437e7c9.png;> any other suggestions for a better solution? ### Are you willing to submit a PR? - [X] I'm willing to submit a PR! -- This is an
[GitHub] [pulsar] liangyepianzhou opened a new pull request, #20261: [fix][broker] Fix `UnsupportedOperationException` when update topic properties.
liangyepianzhou opened a new pull request, #20261: URL: https://github.com/apache/pulsar/pull/20261 ## Motivation The branch master has an NPE check in the `BrokerService::checkOwnershipAndCreatePersistentTopic`, but it will lead to a `UnsupportedOperationException`. The returned map might be an unmodifiable `Collections.emptyMap()`. The call chain involved is as follows: 1. `BrokerService::checkOwnershipAndCreatePersistentTopic` 2. `BrokerService::fetchTopicPropertiesAsync` 3. `ManagedLedgerFactory::getManagedLedgerPropertiesAsync` 4. `MetaStoreImpl::getManagedLedgerPropertiesAsync` 5. `operationFailed` ```java public void operationFailed(MetaStoreException e) { if (e instanceof MetadataNotFoundException) { result.complete(Collections.emptyMap()); } else { result.completeExceptionally(e); } } ``` ### Modifications Make the getManagedLedgerPropertiesAsync return a modifiable map. ### Verifying this change - [ ] Make sure that the change passes the CI checks. *(Please pick either of the following options)* This change is a trivial rework / code cleanup without any test coverage. *(or)* This change is already covered by existing tests, such as *(please describe tests)*. *(or)* This change added tests and can be verified as follows: *(example:)* - *Added integration tests for end-to-end deployment with large payloads (10MB)* - *Extended integration test for recovery after broker failure* ### Does this pull request potentially affect one of the following parts: *If the box was checked, please highlight the changes* - [ ] Dependencies (add or upgrade a dependency) - [ ] The public API - [ ] The schema - [ ] The default values of configurations - [ ] The threading model - [ ] The binary protocol - [ ] The REST endpoints - [ ] The admin CLI options - [ ] The metrics - [ ] Anything that affects deployment ### Documentation - [ ] `doc` - [ ] `doc-required` - [x] `doc-not-needed` - [ ] `doc-complete` ### Matching PR in forked repository PR in forked repository: -- 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] shodo edited a discussion: Difference between JSON schema and AVRO schema
GitHub user shodo edited a discussion: Difference between JSON schema and AVRO schema When checking this section of the doc: https://pulsar.apache.org/docs/3.0.x/schema-understand/ It's not really clear to me what is the difference between a JSON schema and an AVRO schema. When I talk about JSON schema I refer to this specification, basically a JSON to define how a JSON payload is composed: http://json-schema.org/understanding-json-schema/index.html While with avro i refer to this one, a JSON to define how an AVRO payload is composed: https://avro.apache.org/docs/ Both the schemas are written in JSON, but their specifications are quite different. However If i check the Pulsar doc with example in C++ I see that in the AVRO example this string is passed: ```C++ static const std::string exampleSchema = "{\"type\":\"record\",\"name\":\"Example\",\"namespace\":\"test\"," "\"fields\":[{\"name\":\"a\",\"type\":\"int\"},{\"name\":\"b\",\"type\":\"int\"}]}"; Producer producer; ProducerConfiguration producerConf; producerConf.setSchema(SchemaInfo(AVRO, "Avro", exampleSchema)); ``` while in the JSON example: ```C++ Std::string jsonSchema = R"({"type":"record","name":"cpx","fields":[{"name":"re","type":"double"},{"name":"im","type":"double"}]})"; SchemaInfo schemaInfo = SchemaInfo(JSON, "JSON", jsonSchema); ``` Although the two strings are instanced in different ways, the content is pretty similar, and seems they are both respecting the AVRO specification! The only real difference is that the AVRO one has the "namespace" field that is not present in the JSON example. So what's the point of saying that both JSON and AVRO are supported if seems that in both cases the AVRO specification is used? Am I missing something? GitHub link: https://github.com/apache/pulsar/discussions/20260 This is an automatically sent email for commits@pulsar.apache.org. To unsubscribe, please send an email to: commits-unsubscr...@pulsar.apache.org
[GitHub] [pulsar] shodo edited a discussion: Difference between JSON schema and AVRO schema
GitHub user shodo edited a discussion: Difference between JSON schema and AVRO schema When checking this section of the doc: https://pulsar.apache.org/docs/3.0.x/schema-understand/ It's not really clear to me what is the difference between a JSON schema and an AVRO schema. When I talk about JSON schema I refer to this specification, basically a JSON to define how a JSON payload is composed: http://json-schema.org/understanding-json-schema/index.html While with avro i refer to this one, a JSON to define how an AVRO payload is composed: https://avro.apache.org/docs/ Both the schemas are written in JSON, but their specifications are quite different. However If i check the Pulsar doc with example in C++ I see that in the AVRO example this string is passed: ```C++ static const std::string exampleSchema = "{\"type\":\"record\",\"name\":\"Example\",\"namespace\":\"test\"," "\"fields\":[{\"name\":\"a\",\"type\":\"int\"},{\"name\":\"b\",\"type\":\"int\"}]}"; Producer producer; ProducerConfiguration producerConf; producerConf.setSchema(SchemaInfo(AVRO, "Avro", exampleSchema)); ``` while in the JSON example: ```C++ Std::string jsonSchema = R"({"type":"record","name":"cpx","fields":[{"name":"re","type":"double"},{"name":"im","type":"double"}]})"; SchemaInfo schemaInfo = SchemaInfo(JSON, "JSON", jsonSchema); ``` Although the two strings are instanced in different ways, the content is pretty similar, and seems they are both respecting the AVRO specification! So what's the point of saying that both JSON and AVRO are supported if seems that in both cases the AVRO specification is used? Am I missing something? GitHub link: https://github.com/apache/pulsar/discussions/20260 This is an automatically sent email for commits@pulsar.apache.org. To unsubscribe, please send an email to: commits-unsubscr...@pulsar.apache.org
[GitHub] [pulsar] nicoloboschi commented on a diff in pull request #20259: [fix][io] add protobuf ByteString to pulsar-io jdbc core
nicoloboschi commented on code in PR #20259: URL: https://github.com/apache/pulsar/pull/20259#discussion_r1187630848 ## pulsar-io/jdbc/core/pom.xml: ## @@ -58,6 +58,13 @@ com.fasterxml.jackson.dataformat jackson-dataformat-yaml + + + com.google.protobuf + protobuf-java + 3.22.4 Review Comment: +1 we need to use `provided` to avoid runtime clashes Also get the already declared version -- 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] eugene-cheverda commented on pull request #20222: [fix][sec] Update dependencies to use snakeyaml 2.0
eugene-cheverda commented on PR #20222: URL: https://github.com/apache/pulsar/pull/20222#issuecomment-1538583109 Regression changes behavior in the following cases: 1. For regular class, `@Data` annotated the following definition of json property is not serialized in `2.14.2`, gets serialized in `2.15.0`: ```java @JsonIgnore private transient String field4; public String getField4() { return field4; } public void setField4(String field4) { this.field4 = field4; } ``` 2. For `@Value` annotated class the following definitions of json property are not serialized in `2.14.2`, get serialized in `2.15.0`: ```java @JsonIgnore private transient String field3; @JsonIgnore private transient String field4; public String getField4() { return field4; } ``` The most notable discrepancy in behavior between `@Data` and `@Value` annotated classes. The following property will be excluded for `@Data` and will be included in `@Value` class: ```java @JsonIgnore private transient String field3; ``` i.e. for `@Data` annotated class `@JsonIgnore` is being propagated to the generated property, for `@Value` class it is not. Speaking about usages of `@JsonIgnore` and private transient field in pulsar code, please see the list below: - https://github.com/apache/pulsar/blob/e956db729f5098ff319237fd4220dbfb234c1b18/pulsar-common/src/main/java/org/apache/pulsar/client/impl/schema/SchemaInfoImpl.java#L73 - https://github.com/apache/pulsar/blob/e956db729f5098ff319237fd4220dbfb234c1b18/pulsar-client/src/main/java/org/apache/pulsar/client/impl/conf/ClientConfigurationData.java#L62 - https://github.com/apache/pulsar/blob/e956db729f5098ff319237fd4220dbfb234c1b18/pulsar-client/src/main/java/org/apache/pulsar/client/impl/conf/ProducerConfigurationData.java#L177 - https://github.com/apache/pulsar/blob/e956db729f5098ff319237fd4220dbfb234c1b18/pulsar-client/src/main/java/org/apache/pulsar/client/impl/conf/ConsumerConfigurationData.java#L255 - https://github.com/apache/pulsar/blob/e956db729f5098ff319237fd4220dbfb234c1b18/pulsar-client/src/main/java/org/apache/pulsar/client/impl/conf/ConsumerConfigurationData.java#L386 - https://github.com/apache/pulsar/blob/e956db729f5098ff319237fd4220dbfb234c1b18/pulsar-client/src/main/java/org/apache/pulsar/client/impl/conf/ConsumerConfigurationData.java#L395 All of the above occurrences are in `@Data` annotated classes, hence the behavior matches to the one in `2.14.2`, the absence of mentioned properties is covered in tests in PR If this PR gets approved/merged, I'll cherry-pick additional changes into PR agains `branch-3.0` -- 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 a diff in pull request #20259: [fix][io] add protobuf ByteString to pulsar-io jdbc core
tisonkun commented on code in PR #20259: URL: https://github.com/apache/pulsar/pull/20259#discussion_r1187566110 ## pulsar-io/jdbc/core/pom.xml: ## @@ -58,6 +58,13 @@ com.fasterxml.jackson.dataformat jackson-dataformat-yaml + + + com.google.protobuf + protobuf-java + 3.22.4 Review Comment: Let's use the version in `dependencyManagement` in parent pom. Also, cloud we use `provided` scope? (N.B. I don't take a considerate thought). -- 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] bpereto commented on issue #20247: [Bug] Pulsar IO JDBC Clickhouse and Protobuf native with "ByteString" throws exception
bpereto commented on issue #20247: URL: https://github.com/apache/pulsar/issues/20247#issuecomment-1538477433 I submitted the pull request: https://github.com/apache/pulsar/pull/20259 I tested it with clickhouse. Had to enable bytes support via jdbc url: ``` jdbc:clickhouse://192.168.1.50:8123/default?use_binary_string=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
[pulsar] branch master updated: [improve] Make subscriptions on NonPersistentTopic non-durable (#19741)
This is an automated email from the ASF dual-hosted git repository. penghui 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 e956db729f5 [improve] Make subscriptions on NonPersistentTopic non-durable (#19741) e956db729f5 is described below commit e956db729f5098ff319237fd4220dbfb234c1b18 Author: 道君 AuthorDate: Mon May 8 22:41:37 2023 +0800 [improve] Make subscriptions on NonPersistentTopic non-durable (#19741) --- .../nonpersistent/NonPersistentSubscription.java | 24 +--- .../service/nonpersistent/NonPersistentTopic.java | 44 ++- .../apache/pulsar/broker/admin/AdminApi2Test.java | 2 +- .../pulsar/broker/admin/v1/V1_AdminApi2Test.java | 2 +- .../service/ManagedLedgerCompressionTest.java | 2 +- .../broker/service/NonPersistentTopicE2ETest.java | 93 --- .../nonpersistent/NonPersistentTopicTest.java | 132 + .../client/api/NonDurableSubscriptionTest.java | 43 --- .../org/apache/pulsar/client/impl/ReaderTest.java | 2 +- .../apache/pulsar/client/impl/ConsumerImpl.java| 7 ++ .../client/impl/MultiTopicsConsumerImpl.java | 1 + 11 files changed, 231 insertions(+), 121 deletions(-) diff --git a/pulsar-broker/src/main/java/org/apache/pulsar/broker/service/nonpersistent/NonPersistentSubscription.java b/pulsar-broker/src/main/java/org/apache/pulsar/broker/service/nonpersistent/NonPersistentSubscription.java index fc6df4a3409..1048864ad64 100644 --- a/pulsar-broker/src/main/java/org/apache/pulsar/broker/service/nonpersistent/NonPersistentSubscription.java +++ b/pulsar-broker/src/main/java/org/apache/pulsar/broker/service/nonpersistent/NonPersistentSubscription.java @@ -28,6 +28,7 @@ import java.util.concurrent.atomic.AtomicIntegerFieldUpdater; import org.apache.bookkeeper.mledger.Entry; import org.apache.bookkeeper.mledger.Position; import org.apache.bookkeeper.mledger.impl.PositionImpl; +import org.apache.commons.collections4.CollectionUtils; import org.apache.pulsar.broker.intercept.BrokerInterceptor; import org.apache.pulsar.broker.service.AbstractSubscription; import org.apache.pulsar.broker.service.AnalyzeBacklogResult; @@ -65,25 +66,17 @@ public class NonPersistentSubscription extends AbstractSubscription implements S @SuppressWarnings("unused") private volatile int isFenced = FALSE; -// Timestamp of when this subscription was last seen active -private volatile long lastActive; - private volatile Map subscriptionProperties; -// If isDurable is false(such as a Reader), remove subscription from topic when closing this subscription. -private final boolean isDurable; - private KeySharedMode keySharedMode = null; -public NonPersistentSubscription(NonPersistentTopic topic, String subscriptionName, boolean isDurable, +public NonPersistentSubscription(NonPersistentTopic topic, String subscriptionName, Map properties) { this.topic = topic; this.topicName = topic.getName(); this.subName = subscriptionName; this.fullName = MoreObjects.toStringHelper(this).add("topic", topicName).add("name", subName).toString(); IS_FENCED_UPDATER.set(this, FALSE); -this.lastActive = System.currentTimeMillis(); -this.isDurable = isDurable; this.subscriptionProperties = properties != null ? Collections.unmodifiableMap(properties) : Collections.emptyMap(); } @@ -110,7 +103,6 @@ public class NonPersistentSubscription extends AbstractSubscription implements S @Override public synchronized CompletableFuture addConsumer(Consumer consumer) { -updateLastActive(); if (IS_FENCED_UPDATER.get(this) == TRUE) { log.warn("Attempting to add consumer {} on a fenced subscription", consumer); return FutureUtil.failedFuture(new SubscriptionFencedException("Subscription is fenced")); @@ -177,7 +169,6 @@ public class NonPersistentSubscription extends AbstractSubscription implements S @Override public synchronized void removeConsumer(Consumer consumer, boolean isResetCursor) throws BrokerServiceException { -updateLastActive(); if (dispatcher != null) { dispatcher.removeConsumer(consumer); } @@ -185,7 +176,8 @@ public class NonPersistentSubscription extends AbstractSubscription implements S ConsumerStatsImpl stats = consumer.getStats(); bytesOutFromRemovedConsumers.add(stats.bytesOutCounter); msgOutFromRemovedConsumer.add(stats.msgOutCounter); -if (!isDurable) { +// Unsubscribe when all the consumers disconnected. +if (dispatcher != null && CollectionUtils.isEmpty(dispatcher.getConsumers())) { topic.unsubscribe(subName); } @@ -524,14 +516,6 @@ public class
[GitHub] [pulsar] codelipenghui merged pull request #19741: [improve] Make subscriptions on NonPersistentTopic non-durable
codelipenghui merged PR #19741: URL: https://github.com/apache/pulsar/pull/19741 -- 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 #20259: [fix][io] add protobuf ByteString to pulsar-io jdbc core
github-actions[bot] commented on PR #20259: URL: https://github.com/apache/pulsar/pull/20259#issuecomment-1538473228 @bpereto Please add the following content to your PR description and select a checkbox: ``` - [ ] `doc` - [ ] `doc-required` - [ ] `doc-not-needed` - [ ] `doc-complete` ``` -- 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] bpereto opened a new pull request, #20259: [fix][io] add protobuf ByteString to pulsar-io jdbc core
bpereto opened a new pull request, #20259: URL: https://github.com/apache/pulsar/pull/20259 - tested with jdbc clickhouse on pulsar 2.11.1 Fixes #20247 ### Motivation ### Modifications ### Verifying this change - [ ] Make sure that the change passes the CI checks. *(Please pick either of the following options)* This change is a trivial rework / code cleanup without any test coverage. *(or)* This change is already covered by existing tests, such as *(please describe tests)*. *(or)* This change added tests and can be verified as follows: *(example:)* - *Added integration tests for end-to-end deployment with large payloads (10MB)* - *Extended integration test for recovery after broker failure* ### Does this pull request potentially affect one of the following parts: *If the box was checked, please highlight the changes* - [x] Dependencies (add or upgrade a dependency) - [ ] The public API - [ ] The schema - [ ] The default values of configurations - [ ] The threading model - [ ] The binary protocol - [ ] The REST endpoints - [ ] The admin CLI options - [ ] The metrics - [ ] Anything that affects deployment ### Documentation - [ ] `doc` - [ ] `doc-required` - [ ] `doc-not-needed` - [ ] `doc-complete` ### Matching PR in forked repository PR in forked repository: -- 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 pull request #20230: [fix][monitor] topic with double quote breaks the prometheus format
mattisonchao commented on PR #20230: URL: https://github.com/apache/pulsar/pull/20230#issuecomment-1538458598 Wierd topic name, it should be illegal after applying PIP https://github.com/apache/pulsar/issues/19239. -- 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 #20238: [feat][ws] Use async auth method to support OIDC
mattisonchao commented on code in PR #20238: URL: https://github.com/apache/pulsar/pull/20238#discussion_r1187515180 ## pulsar-broker-common/src/main/java/org/apache/pulsar/broker/authentication/AuthenticationService.java: ## @@ -171,20 +172,26 @@ public String authenticateHttpRequest(HttpServletRequest request, Authentication authData = authenticationState.getAuthDataSource(); } // Backward compatible, the authData value was null in the previous implementation -return providerToUse.authenticate(authData); +return providerToUse.authenticateAsync(authData).get(); } catch (AuthenticationException e) { if (LOG.isDebugEnabled()) { LOG.debug("Authentication failed for provider " + providerToUse.getAuthMethodName() + " : " + e.getMessage(), e); } throw e; +} catch (ExecutionException | InterruptedException e) { +if (LOG.isDebugEnabled()) { +LOG.debug("Authentication failed for provider " + providerToUse.getAuthMethodName() + " : " ++ e.getMessage(), e); +} +throw new RuntimeException(e); Review Comment: I am unsure who will use it. Should we consider remark interrupted in the current 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] mattisonchao commented on a diff in pull request #20238: [feat][ws] Use async auth method to support OIDC
mattisonchao commented on code in PR #20238: URL: https://github.com/apache/pulsar/pull/20238#discussion_r1187513566 ## pulsar-broker-common/src/main/java/org/apache/pulsar/broker/authentication/AuthenticationService.java: ## @@ -171,20 +172,26 @@ public String authenticateHttpRequest(HttpServletRequest request, Authentication authData = authenticationState.getAuthDataSource(); } // Backward compatible, the authData value was null in the previous implementation -return providerToUse.authenticate(authData); +return providerToUse.authenticateAsync(authData).get(); Review Comment: Can we give it a timeout by any chance? -- 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] nicoloboschi commented on pull request #20230: [fix][monitor] topic with double quote breaks the prometheus format
nicoloboschi commented on PR #20230: URL: https://github.com/apache/pulsar/pull/20230#issuecomment-1538440592 /pulsarbot rerun-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 #20230: [fix][monitor] topic with double quote breaks the prometheus format
michaeljmarshall commented on code in PR #20230: URL: https://github.com/apache/pulsar/pull/20230#discussion_r1187485547 ## pulsar-broker/src/main/java/org/apache/pulsar/broker/stats/prometheus/PrometheusMetricStreams.java: ## @@ -41,7 +41,11 @@ void writeSample(String metricName, Number value, String... labelsAndValuesArray SimpleTextOutputStream stream = initGaugeType(metricName); stream.write(metricName).write('{'); for (int i = 0; i < labelsAndValuesArray.length; i += 2) { - stream.write(labelsAndValuesArray[i]).write("=\"").write(labelsAndValuesArray[i + 1]).write('\"'); +String labelValue = labelsAndValuesArray[i + 1]; +if (labelValue != null) { +labelValue = labelValue.replace("\"", "\\\""); Review Comment: Of course, thanks for the explanation @pgier and @nicoloboschi. I was mixing up my terms. -- 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] maraiskruger1980 commented on pull request #805: [Issue 456] Support chunking for big messages.
maraiskruger1980 commented on PR #805: URL: https://github.com/apache/pulsar-client-go/pull/805#issuecomment-1538408207 That will be great if it can support shared subscription -- 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] nicoloboschi commented on a diff in pull request #20230: [fix][monitor] topic with double quote breaks the prometheus format
nicoloboschi commented on code in PR #20230: URL: https://github.com/apache/pulsar/pull/20230#discussion_r1187451013 ## pulsar-broker/src/main/java/org/apache/pulsar/broker/stats/prometheus/PrometheusMetricStreams.java: ## @@ -41,7 +41,11 @@ void writeSample(String metricName, Number value, String... labelsAndValuesArray SimpleTextOutputStream stream = initGaugeType(metricName); stream.write(metricName).write('{'); for (int i = 0; i < labelsAndValuesArray.length; i += 2) { - stream.write(labelsAndValuesArray[i]).write("=\"").write(labelsAndValuesArray[i + 1]).write('\"'); +String labelValue = labelsAndValuesArray[i + 1]; +if (labelValue != null) { +labelValue = labelValue.replace("\"", "\\\""); Review Comment: yes I tested it in Prometheus. In the target section you get this error >expected equal, got "\"" ("INVALID") while parsing: "pulsar_storage_logical_size{cluster=\"standalone\",namespace=\"public/default\",topic=\"persistent://public/default/\"topic\"" Now the error doesn't show up and the metric is queryable from both Prometheus and Grafana -- 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] hpvd added a comment to the discussion: Pulsar Shell: native image with GraalVM
GitHub user hpvd added a comment to the discussion: Pulsar Shell: native image with GraalVM perfect. thanks! GitHub link: https://github.com/apache/pulsar/discussions/20095#discussioncomment-5838322 This is an automatically sent email for commits@pulsar.apache.org. To unsubscribe, please send an email to: commits-unsubscr...@pulsar.apache.org
[GitHub] [pulsar] nicoloboschi commented on a diff in pull request #20230: [fix][monitor] topic with double quote breaks the prometheus format
nicoloboschi commented on code in PR #20230: URL: https://github.com/apache/pulsar/pull/20230#discussion_r1187451013 ## pulsar-broker/src/main/java/org/apache/pulsar/broker/stats/prometheus/PrometheusMetricStreams.java: ## @@ -41,7 +41,11 @@ void writeSample(String metricName, Number value, String... labelsAndValuesArray SimpleTextOutputStream stream = initGaugeType(metricName); stream.write(metricName).write('{'); for (int i = 0; i < labelsAndValuesArray.length; i += 2) { - stream.write(labelsAndValuesArray[i]).write("=\"").write(labelsAndValuesArray[i + 1]).write('\"'); +String labelValue = labelsAndValuesArray[i + 1]; +if (labelValue != null) { +labelValue = labelValue.replace("\"", "\\\""); Review Comment: yes I tested it in Prometheus. In the target section you get this error >expected equal, got "\"" ("INVALID") while parsing: "pulsar_storage_logical_size{cluster=\"standalone\",namespace=\"public/default\",topic=\"persistent://public/default/\"topic\"" -- 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] zbentley opened a new issue, #20258: [Bug] Topic "peek" (admin/v2/persistent/{tenant}/{namespace}/{topic}/ledger/{ledgerId}/entry/{entryId}) management API returns undocumented and i
zbentley opened a new issue, #20258: URL: https://github.com/apache/pulsar/issues/20258 ### Search before asking - [X] I searched in the [issues](https://github.com/apache/pulsar/issues) and found nothing similar. ### Version 2.10.3 ### Minimal reproduce step 1. Produce a message containing a bytes-schema payload `abc123` to a persistent topic with a *non-batched* (`batchingEnabled=false`) producer. 2. Retrieve that message from the topic using the v2 admin API's `GET admin/v2/persistent/{tenant}/{namespace}/{topic}/ledger/{ledgerId}/entry/{entryId}` functionality. 3. Observe that the returned payload exactly matches the string `abc123`. 4. Produce the exact same message with `batchingEnabled=true`. 5. Repeat step 2. 6. Observe that the returned payload no longer matches `abc123`. ### What did you expect to see? Something I can use to extract individual messages from a batch. That could include any of the below ideas, or something totally different: - (Easiest) documentation on how to parse a batch of messages client-side such that I can extract an individual message. I checked `PulsarApi.proto` against the batch data and nothing added up, so I suspect this is something internal, but may be wrong. - A change to the API to support a parameter indicating batch message index, such that only messages with that index would be returned. - Variable length HTTP headers indicating where each message content lies (offset and length) within the returned batch blob. - A change to the API to return multipart HTTP responses, one per batch message. ### What did you see instead? An undocumented blob of binary (I think this is a raw chunk of message data from the ledger) that looks like it contains some info re: properties/etc. up front, and then concatenated message+metadata entries for each message in the batch. ### Anything else? Many of my proposed fixes break backwards compatibility, so this may be better suited as a feature request. However, in the short term, I'd love to find a reference on how to extract individual messages from the batch in a non-Java environment. I control all my admin API accesses in my environment, so I can add parsing logic to those wrappers--I just need to know how to parse the data. ### Are you willing to submit a PR? - [X] I'm willing to submit a 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.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [pulsar-client-go] Gleiphir2769 commented on pull request #805: [Issue 456] Support chunking for big messages.
Gleiphir2769 commented on PR #805: URL: https://github.com/apache/pulsar-client-go/pull/805#issuecomment-1538335263 > Does the chunking work on shared subscription introduced in Pulsar 2.11 Hi @maraiskruger1980, This PR is finished when pulsar 2.11 has not been released. So it doesn't support shared subscription chunking. I think I can take some time on it. Welcome to follow the progress. -- 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-site] Anonymitaet commented on a diff in pull request #543: Add doc for transform functions before sinks
Anonymitaet commented on code in PR #543: URL: https://github.com/apache/pulsar-site/pull/543#discussion_r1187386622 ## docs/io-use.md: ## @@ -466,6 +466,14 @@ For the latest and complete information, see [Pulsar admin docs](pathname:///ref +## Run a Pulsar Function before a sink connector + +You have the possibility to run a [Pulsar Function](functions-overview.md) in-memory before a sink connector (see [PIP-193](https://github.com/apache/pulsar/issues/16739)). Review Comment: ```suggestion You can run a [Pulsar Function](functions-overview.md) in memory before a sink connector. For details, see [[PIP 193: Sink preprocessing Function](https://github.com/apache/pulsar/issues/16739). ``` ## docs/io-use.md: ## @@ -466,6 +466,14 @@ For the latest and complete information, see [Pulsar admin docs](pathname:///ref +## Run a Pulsar Function before a sink connector + +You have the possibility to run a [Pulsar Function](functions-overview.md) in-memory before a sink connector (see [PIP-193](https://github.com/apache/pulsar/issues/16739)). +Running a Pulsar Function in memory before a sink connector provides lower latency, less I/O and disk consumption than going through an intermediate topic. Review Comment: ```suggestion Running a Pulsar Function in memory before a sink connector provides lower latency, less I/O, and disk consumption than going through an intermediate topic. ``` -- 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 opened a new pull request, #20257: [fix][broker] Fix ledger cachemiss size
AnonHxy opened a new pull request, #20257: URL: https://github.com/apache/pulsar/pull/20257 ### Motivation ledger cache miss rate should be same as ledgerFactory cache miss rate ### Modifications ledger cache miss rate should be same as ledgerFactory cache miss rate ### Verifying this change - [x] Make sure that the change passes the CI checks. ### Documentation - [ ] `doc` - [ ] `doc-required` - [x] `doc-not-needed` - [ ] `doc-complete` ### Matching PR in forked repository PR in forked repository: -- 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-site] Anonymitaet commented on pull request #543: Add doc for transform functions before sinks
Anonymitaet commented on PR #543: URL: https://github.com/apache/pulsar-site/pull/543#issuecomment-1538273126 @momo-jun OK, thank you! -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: commits-unsubscr...@pulsar.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [pulsar-site] Anonymitaet commented on pull request #555: Document OpenID Connect Auth Provider
Anonymitaet commented on PR #555: URL: https://github.com/apache/pulsar-site/pull/555#issuecomment-1538267870 @momo-jun OK, thank you! -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: commits-unsubscr...@pulsar.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [pulsar-site] Anonymitaet commented on pull request #544: [improve][fn] add docs for supporting reading config options from files in Python runner
Anonymitaet commented on PR #544: URL: https://github.com/apache/pulsar-site/pull/544#issuecomment-1538266179 @tisonkun Oops! Thank you! -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: commits-unsubscr...@pulsar.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [pulsar-client-node] XXXMrG commented on pull request #322: [feat]: Support client.getPartitionsForTopic (#320)
XXXMrG commented on PR #322: URL: https://github.com/apache/pulsar-client-node/pull/322#issuecomment-1538259510 @shibd I have fixed the compile problem in Windows x86 platforms. But I found that the `Client/getPartitionsForTopic` test will fail in the CI environment due to an unexpected extra item that duplicates the topic name in the returned result, it does not happened in my device, this may be related to the Pulsar version in the CI environment, which is `2.10.2`, and i use pulsar `2.11.x`. I need some assistance to resolve this issue, thx. -- 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 closed issue #20234: [Bug] JavaDoc (generated?) has Chinese text in it (as appears in Intellij)
tisonkun closed issue #20234: [Bug] JavaDoc (generated?) has Chinese text in it (as appears in Intellij) URL: https://github.com/apache/pulsar/issues/20234 -- 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 #20234: [Bug] JavaDoc (generated?) has Chinese text in it (as appears in Intellij)
tisonkun commented on issue #20234: URL: https://github.com/apache/pulsar/issues/20234#issuecomment-1538239253 Closing... Invalid. -- 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: [fix][client] Seek should be thread-safe (#20242)
This is an automated email from the ASF dual-hosted git repository. tison 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 bc1764f9ef7 [fix][client] Seek should be thread-safe (#20242) bc1764f9ef7 is described below commit bc1764f9ef71dd31e8cd61c7571e493442bc6395 Author: Kim, Joo Hyuk AuthorDate: Mon May 8 20:57:20 2023 +0900 [fix][client] Seek should be thread-safe (#20242) --- .../apache/pulsar/client/impl/ConsumerImpl.java| 109 - .../pulsar/client/impl/ConsumerImplTest.java | 27 - 2 files changed, 86 insertions(+), 50 deletions(-) diff --git a/pulsar-client/src/main/java/org/apache/pulsar/client/impl/ConsumerImpl.java b/pulsar-client/src/main/java/org/apache/pulsar/client/impl/ConsumerImpl.java index 199e8a9ae71..6c64fe04069 100644 --- a/pulsar-client/src/main/java/org/apache/pulsar/client/impl/ConsumerImpl.java +++ b/pulsar-client/src/main/java/org/apache/pulsar/client/impl/ConsumerImpl.java @@ -208,6 +208,7 @@ public class ConsumerImpl extends ConsumerBase implements ConnectionHandle private final AtomicReference clientCnxUsedForConsumerRegistration = new AtomicReference<>(); private final List previousExceptions = new CopyOnWriteArrayList(); + static ConsumerImpl newConsumerImpl(PulsarClientImpl client, String topic, ConsumerConfigurationData conf, @@ -251,10 +252,12 @@ public class ConsumerImpl extends ConsumerBase implements ConnectionHandle } protected ConsumerImpl(PulsarClientImpl client, String topic, ConsumerConfigurationData conf, - ExecutorProvider executorProvider, int partitionIndex, boolean hasParentConsumer, - boolean parentConsumerHasListener, CompletableFuture> subscribeFuture, MessageId startMessageId, - long startMessageRollbackDurationInSec, Schema schema, ConsumerInterceptors interceptors, - boolean createTopicIfDoesNotExist) { + ExecutorProvider executorProvider, int partitionIndex, boolean hasParentConsumer, + boolean parentConsumerHasListener, CompletableFuture> subscribeFuture, + MessageId startMessageId, + long startMessageRollbackDurationInSec, Schema schema, + ConsumerInterceptors interceptors, + boolean createTopicIfDoesNotExist) { super(client, topic, conf, conf.getReceiverQueueSize(), executorProvider, subscribeFuture, schema, interceptors); this.consumerId = client.newConsumerId(); @@ -319,21 +322,21 @@ public class ConsumerImpl extends ConsumerBase implements ConnectionHandle } this.connectionHandler = new ConnectionHandler(this, -new BackoffBuilder() - .setInitialTime(client.getConfiguration().getInitialBackoffIntervalNanos(), -TimeUnit.NANOSECONDS) - .setMax(client.getConfiguration().getMaxBackoffIntervalNanos(), TimeUnit.NANOSECONDS) -.setMandatoryStop(0, TimeUnit.MILLISECONDS) -.create(), +new BackoffBuilder() + .setInitialTime(client.getConfiguration().getInitialBackoffIntervalNanos(), +TimeUnit.NANOSECONDS) + .setMax(client.getConfiguration().getMaxBackoffIntervalNanos(), TimeUnit.NANOSECONDS) +.setMandatoryStop(0, TimeUnit.MILLISECONDS) +.create(), this); this.topicName = TopicName.get(topic); if (this.topicName.isPersistent()) { this.acknowledgmentsGroupingTracker = -new PersistentAcknowledgmentsGroupingTracker(this, conf, client.eventLoopGroup()); +new PersistentAcknowledgmentsGroupingTracker(this, conf, client.eventLoopGroup()); } else { this.acknowledgmentsGroupingTracker = -NonPersistentAcknowledgmentGroupingTracker.of(); +NonPersistentAcknowledgmentGroupingTracker.of(); } if (conf.getDeadLetterPolicy() != null) { @@ -410,16 +413,16 @@ public class ConsumerImpl extends ConsumerBase implements ConnectionHandle log.error("[{}][{}] Failed to unsubscribe: {}", topic, subscription, e.getCause().getMessage()); setState(State.Ready); unsubscribeFuture.completeExceptionally( -PulsarClientException.wrap(e.getCause(), -String.format("Failed to unsubscribe the subscription %s of topic %s", -
[GitHub] [pulsar] tisonkun merged pull request #20242: [fix][client] Java Client's Seek Logic Not Threadsafe #1
tisonkun merged PR #20242: URL: https://github.com/apache/pulsar/pull/20242 -- 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 #20242: [fix][client] Java Client's Seek Logic Not Threadsafe #1
tisonkun commented on PR #20242: URL: https://github.com/apache/pulsar/pull/20242#issuecomment-1538237232 Never mind. I can edit the commit message before merging :D -- 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 closed issue #19671: [Bug] Java Client's Seek Logic Not Threadsafe
tisonkun closed issue #19671: [Bug] Java Client's Seek Logic Not Threadsafe URL: https://github.com/apache/pulsar/issues/19671 -- 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 #15795: 在配置ns-isolation-policy 的时候broker的配置不能采用hostname:端口
tisonkun commented on issue #15795: URL: https://github.com/apache/pulsar/issues/15795#issuecomment-1538227009 Hi @hzluyang! I've open a PR in https://github.com/apache/pulsar/pull/20256 which you can test out. -- 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, #20256: [feat][broker] ns-isolation-policy support hostport pattern
tisonkun opened a new pull request, #20256: URL: https://github.com/apache/pulsar/pull/20256 This closes #15795. ### Motivation `ns-isolation-policy` primary and secondary regex pattern currently verify against hostname only, while we return broker list in hostport pattern, we may support the regex pattern test against the whole URL (or a combine of `host:port` but that can be a bit tricky. The direction is still to be determinated IMO. ### Modifications AS WHAT MENTIONED IN MOTIVATION ### Verifying this change New tests added. ### Does this pull request potentially affect one of the following parts: *If the box was checked, please highlight the changes* - [ ] Dependencies (add or upgrade a dependency) - [ ] The public API - [ ] The schema - [ ] The default values of configurations - [ ] The threading model - [ ] The binary protocol - [ ] The REST endpoints - [ ] The admin CLI options - [ ] The metrics - [ ] Anything that affects deployment ### Documentation - [ ] `doc` - [ ] `doc-required` - [ ] `doc-not-needed` - [ ] `doc-complete` ### Matching PR in forked repository PR in forked repository: -- 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] JooHyukKim commented on pull request #20242: [fix][client] Java Client's Seek Logic Not Threadsafe #1
JooHyukKim commented on PR #20242: URL: https://github.com/apache/pulsar/pull/20242#issuecomment-1538222368 > @JooHyukKim May I ask what `#1` means? Oh... "#1" means My first ever contribution to Pulsar! I can remove the "#1" part from title if you think it will confuse the maintenance team. Just let me know~ @tisonkun > Thanks for your contribution! @tisonkun Thank you for great guidance on your part -- 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 #9224: [Pulsar SQL] Some optimized points in PR 8422
tisonkun commented on issue #9224: URL: https://github.com/apache/pulsar/issues/9224#issuecomment-1538220115 @ziang123 Thanks for your contribution! -- 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 closed issue #9224: [Pulsar SQL] Some optimized points in PR 8422
tisonkun closed issue #9224: [Pulsar SQL] Some optimized points in PR 8422 URL: https://github.com/apache/pulsar/issues/9224 -- 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 #9224: [Pulsar SQL] Some optimized points in PR 8422
tisonkun commented on issue #9224: URL: https://github.com/apache/pulsar/issues/9224#issuecomment-1538219693 Closed by #19027. -- 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 (2f9f5df4a2b -> 3c80af2df9c)
This is an automated email from the ASF dual-hosted git repository. tison pushed a change to branch master in repository https://gitbox.apache.org/repos/asf/pulsar.git from 2f9f5df4a2b [fix][broker] Fix `RoaringBitmap.contains` can't check value 65535 (#20176) add 3c80af2df9c [improve][sql] Reuse factory and replace if-else with switch (#19027) No new revisions were added by this update. Summary of changes: .../apache/pulsar/common/schema/SchemaType.java| 1 - .../presto/PulsarDispatchingRowDecoderFactory.java | 47 -- .../pulsar/sql/presto/PulsarRecordCursor.java | 71 -- 3 files changed, 69 insertions(+), 50 deletions(-)
[GitHub] [pulsar] tisonkun merged pull request #19027: [improve][sql] Some optimized points in #9224
tisonkun merged PR #19027: URL: https://github.com/apache/pulsar/pull/19027 -- 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] codecov-commenter commented on pull request #20242: [fix][client] Java Client's Seek Logic Not Threadsafe #1
codecov-commenter commented on PR #20242: URL: https://github.com/apache/pulsar/pull/20242#issuecomment-1538216007 ## [Codecov](https://app.codecov.io/gh/apache/pulsar/pull/20242?src=pr=h1_medium=referral_source=github_content=comment_campaign=pr+comments_term=The+Apache+Software+Foundation) Report > Merging [#20242](https://app.codecov.io/gh/apache/pulsar/pull/20242?src=pr=desc_medium=referral_source=github_content=comment_campaign=pr+comments_term=The+Apache+Software+Foundation) (4075a39) into [master](https://app.codecov.io/gh/apache/pulsar/commit/98413642995eb6e562f6a591dcf56e20ac0cc7ef?el=desc_medium=referral_source=github_content=comment_campaign=pr+comments_term=The+Apache+Software+Foundation) (9841364) will **decrease** coverage by `0.02%`. > The diff coverage is `51.42%`. [![Impacted file tree graph](https://app.codecov.io/gh/apache/pulsar/pull/20242/graphs/tree.svg?width=650=150=pr=acYqCpsK9J_medium=referral_source=github_content=comment_campaign=pr+comments_term=The+Apache+Software+Foundation)](https://app.codecov.io/gh/apache/pulsar/pull/20242?src=pr=tree_medium=referral_source=github_content=comment_campaign=pr+comments_term=The+Apache+Software+Foundation) ```diff @@ Coverage Diff @@ ## master #20242 +/- ## - Coverage 72.87% 72.86% -0.02% + Complexity31943 3601 -28342 Files 1868 1868 Lines138594 138597 +3 Branches 1524615247 +1 - Hits 101003 100991 -12 - Misses2954729557 +10 - Partials 8044 8049 +5 ``` | Flag | Coverage Δ | | |---|---|---| | inttests | `24.21% <22.85%> (-0.05%)` | :arrow_down: | | systests | `24.78% <20.00%> (-0.15%)` | :arrow_down: | | unittests | `72.18% <51.42%> (+0.01%)` | :arrow_up: | Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags?utm_medium=referral_source=github_content=comment_campaign=pr+comments_term=The+Apache+Software+Foundation#carryforward-flags-in-the-pull-request-comment) to find out more. | [Impacted Files](https://app.codecov.io/gh/apache/pulsar/pull/20242?src=pr=tree_medium=referral_source=github_content=comment_campaign=pr+comments_term=The+Apache+Software+Foundation) | Coverage Δ | | |---|---|---| | [...va/org/apache/pulsar/client/impl/ConsumerImpl.java](https://app.codecov.io/gh/apache/pulsar/pull/20242?src=pr=tree_medium=referral_source=github_content=comment_campaign=pr+comments_term=The+Apache+Software+Foundation#diff-cHVsc2FyLWNsaWVudC9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcHVsc2FyL2NsaWVudC9pbXBsL0NvbnN1bWVySW1wbC5qYXZh) | `76.40% <51.42%> (-0.03%)` | :arrow_down: | ... and [64 files with indirect coverage changes](https://app.codecov.io/gh/apache/pulsar/pull/20242/indirect-changes?src=pr=tree-more_medium=referral_source=github_content=comment_campaign=pr+comments_term=The+Apache+Software+Foundation) -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: commits-unsubscr...@pulsar.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [pulsar] tisonkun commented on pull request #19027: [improve][sql] Some optimized points in #9224
tisonkun commented on PR #19027: URL: https://github.com/apache/pulsar/pull/19027#issuecomment-1538218437 Merging... -- 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 #20242: [fix][client] Java Client's Seek Logic Not Threadsafe #1
tisonkun commented on PR #20242: URL: https://github.com/apache/pulsar/pull/20242#issuecomment-1538218263 @JooHyukKim May I ask what `#1` means? It looks to me as part one or first attempt that I may wonder if you have outstanding works on this topic. -- 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 #20242: [fix][client] Java Client's Seek Logic Not Threadsafe #1
tisonkun commented on PR #20242: URL: https://github.com/apache/pulsar/pull/20242#issuecomment-1538217675 Merging... Thanks for your contribution! -- 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 added a comment to the discussion: Cancel delayed message
GitHub user codelipenghui added a comment to the discussion: Cancel delayed message The issue had no activity for 30 days, mark with Stale label. GitHub link: https://github.com/apache/pulsar/discussions/20255#discussioncomment-5837006 This is an automatically sent email for commits@pulsar.apache.org. To unsubscribe, please send an email to: commits-unsubscr...@pulsar.apache.org
[GitHub] [pulsar] tisonkun added a comment to the discussion: Cancel delayed message
GitHub user tisonkun added a comment to the discussion: Cancel delayed message @HarryFQ What do you mean by "cancel"? Do you want the broker to cancel delay messages that are not consumed or the client to do something? GitHub link: https://github.com/apache/pulsar/discussions/20255#discussioncomment-5837015 This is an automatically sent email for commits@pulsar.apache.org. To unsubscribe, please send an email to: commits-unsubscr...@pulsar.apache.org
[GitHub] [pulsar] HarryFQ created a discussion: Cancel delayed message
GitHub user HarryFQ created a discussion: Cancel delayed message **Is your feature request related to a problem? Please describe.** Now only add a delayed message above the delayed message, there is no function to cancel the delayed message. We have about 40-50 million delayed messages every day, and currently we are using redis zset to add and cancel. **Describe the solution you'd like** Delayed messages that are not consumed can be cancelled. **Describe alternatives you've considered** A clear and concise description of any alternative solutions or features you've considered. **Additional context** Add any other context or screenshots about the feature request here. GitHub link: https://github.com/apache/pulsar/discussions/20255 This is an automatically sent email for commits@pulsar.apache.org. To unsubscribe, please send an email to: commits-unsubscr...@pulsar.apache.org
[GitHub] [pulsar] tisonkun closed issue #11180: Cancel delayed message
tisonkun closed issue #11180: Cancel delayed message URL: https://github.com/apache/pulsar/issues/11180 -- 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 closed issue #15794: non-persistent的topic 在消费能力不够出现堆积的时候限制使用内存的大小放置OOM
tisonkun closed issue #15794: non-persistent的topic 在消费能力不够出现堆积的时候限制使用内存的大小放置OOM URL: https://github.com/apache/pulsar/issues/15794 -- 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 #15794: non-persistent的topic 在消费能力不够出现堆积的时候限制使用内存的大小放置OOM
tisonkun commented on issue #15794: URL: https://github.com/apache/pulsar/issues/15794#issuecomment-1538178281 Close as stale and it seems little interest and no volunteer wants to do 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-site] tisonkun merged pull request #563: Delete yarn.lock
tisonkun merged PR #563: URL: https://github.com/apache/pulsar-site/pull/563 -- 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 tisonkun-patch-1 deleted (was b117f1027da)
This is an automated email from the ASF dual-hosted git repository. tison pushed a change to branch tisonkun-patch-1 in repository https://gitbox.apache.org/repos/asf/pulsar-site.git was b117f1027da Delete yarn.lock The revisions that were on this branch are still contained in other references; therefore, this change does not discard any commits from the repository.
[GitHub] [pulsar] codelipenghui commented on pull request #19882: [fix][broker] Fix broker is not able to load topic with broken schema ledger
codelipenghui commented on PR #19882: URL: https://github.com/apache/pulsar/pull/19882#issuecomment-1538152284 @rdhabalia Could you please take a look at @poorbarcode 's 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] JooHyukKim commented on pull request #20242: [fix][client] Java Client's Seek Logic Not Threadsafe #1
JooHyukKim commented on PR #20242: URL: https://github.com/apache/pulsar/pull/20242#issuecomment-1538137187 /pulsarbot rerun-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] tisonkun added a comment to the discussion: Pulsar Shell: native image with GraalVM
GitHub user tisonkun added a comment to the discussion: Pulsar Shell: native image with GraalVM Never mind. I manually remove it. GitHub link: https://github.com/apache/pulsar/discussions/20095#discussioncomment-5836748 This is an automatically sent email for commits@pulsar.apache.org. To unsubscribe, please send an email to: commits-unsubscr...@pulsar.apache.org
[GitHub] [pulsar] hpvd added a comment to the discussion: Build distroless package for better security, smaller size, speed and more
GitHub user hpvd added a comment to the discussion: Build distroless package for better security, smaller size, speed and more hmm maybe distroless is not the only suitable approach to pay-in on goals named above for distroless coming from related https://github.com/apache/pulsar/discussions/20095#discussioncomment-5836331 Example from the other side: Kafka broker (and Zookeeper) compiled to native using Quarkus and GraalVM https://github.com/ozangunalp/kafka-native GitHub link: https://github.com/apache/pulsar/discussions/20253#discussioncomment-5836467 This is an automatically sent email for commits@pulsar.apache.org. To unsubscribe, please send an email to: commits-unsubscr...@pulsar.apache.org
[GitHub] [pulsar] hpvd edited a comment on the discussion: Pulsar Shell: native image with GraalVM
GitHub user hpvd edited a comment on the discussion: Pulsar Shell: native image with GraalVM @tisonkun when making a discussion from an issue, comments in discussion seem not to remove the stale label automatically set before in the issue GitHub link: https://github.com/apache/pulsar/discussions/20095#discussioncomment-5836410 This is an automatically sent email for commits@pulsar.apache.org. To unsubscribe, please send an email to: commits-unsubscr...@pulsar.apache.org
[GitHub] [pulsar] hpvd added a comment to the discussion: Pulsar Shell: native image with GraalVM
GitHub user hpvd added a comment to the discussion: Pulsar Shell: native image with GraalVM @tisonkun when making a discussion from an issue, comments in discussion seem not to remove the stale label (automatically set before in the issue) GitHub link: https://github.com/apache/pulsar/discussions/20095#discussioncomment-5836410 This is an automatically sent email for commits@pulsar.apache.org. To unsubscribe, please send an email to: commits-unsubscr...@pulsar.apache.org
[GitHub] [pulsar] hpvd added a comment to the discussion: Pulsar Shell: native image with GraalVM
GitHub user hpvd added a comment to the discussion: Pulsar Shell: native image with GraalVM and if this "native" approach is thinkable, it may have a strong relationship to addressing the targets from Build distroless package for better security, smaller size, speed and more #20253 GitHub link: https://github.com/apache/pulsar/discussions/20095#discussioncomment-5836377 This is an automatically sent email for commits@pulsar.apache.org. To unsubscribe, please send an email to: commits-unsubscr...@pulsar.apache.org