You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@pulsar.apache.org by GitBox <gi...@apache.org> on 2022/06/06 01:04:58 UTC
[GitHub] [pulsar] mattisonchao opened a new pull request, #15942: Support HTTP endpoint consume messages
mattisonchao opened a new pull request, #15942:
URL: https://github.com/apache/pulsar/pull/15942
### Motivation
According to [PIP-64](https://github.com/apache/pulsar/wiki/PIP-64%3A-Introduce-REST-endpoints-for-producing%2C-consuming-and-reading-messages), We have to support consume messages by HTTP endpoint.
### Modifications
This pull request supports some endpoint for the persistent topic.
- Support Create consumer by HTTP endpoint.
- Support Delete consumer by HTTP endpoint.
- Support Consume messages by HTTP endpoint.
- Support Acknowledges messages by HTTP endpoint.
### Verifying this change
- [x] Make sure that the change passes the CI checks.
### Documentation
- [x] `doc-not-needed`
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] lhotari commented on a diff in pull request #15942: [feature][broker] Support HTTP endpoint consume messages
Posted by GitBox <gi...@apache.org>.
lhotari commented on code in PR #15942:
URL: https://github.com/apache/pulsar/pull/15942#discussion_r889871077
##########
pulsar-broker/src/main/java/org/apache/pulsar/broker/rest/Topics.java:
##########
@@ -157,4 +164,140 @@ public void produceOnNonPersistentTopicPartition(@Suspended final AsyncResponse
}
}
+ @POST
+ @Path("/persistent/{tenant}/{namespace}/{topic}/subscription/{subscription}")
Review Comment:
Shouldn't the API path contain end with "/consumers" if this API method creates a consumer?
```suggestion
@Path("/persistent/{tenant}/{namespace}/{topic}/subscription/{subscription}/consumers")
```
##########
pulsar-broker/src/main/java/org/apache/pulsar/broker/rest/Topics.java:
##########
@@ -157,4 +164,140 @@ public void produceOnNonPersistentTopicPartition(@Suspended final AsyncResponse
}
}
+ @POST
+ @Path("/persistent/{tenant}/{namespace}/{topic}/subscription/{subscription}")
Review Comment:
Shouldn't the API path end with "/consumers" if this API method creates a consumer?
```suggestion
@Path("/persistent/{tenant}/{namespace}/{topic}/subscription/{subscription}/consumers")
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] mattisonchao commented on pull request #15942: [feature][broker] Support HTTP endpoint consume messages
Posted by GitBox <gi...@apache.org>.
mattisonchao commented on PR #15942:
URL: https://github.com/apache/pulsar/pull/15942#issuecomment-1150786552
We chose another way to implement it, and Please see the mail list: `https://lists.apache.org/list.html?dev@pulsar.apache.org.`
So, close this PR.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] cbornet commented on pull request #15942: [feature][broker] Support HTTP endpoint consume messages
Posted by "cbornet (via GitHub)" <gi...@apache.org>.
cbornet commented on PR #15942:
URL: https://github.com/apache/pulsar/pull/15942#issuecomment-1410768789
Not completely the same but note that we now have a Sink to an HTTP endpoint : https://pulsar.apache.org/docs/2.11.x/io-http-sink/
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] mattisonchao closed pull request #15942: [feature][broker] Support HTTP endpoint consume messages
Posted by GitBox <gi...@apache.org>.
mattisonchao closed pull request #15942: [feature][broker] Support HTTP endpoint consume messages
URL: https://github.com/apache/pulsar/pull/15942
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] mattisonchao commented on a diff in pull request #15942: [feature][broker] Support HTTP endpoint consume messages
Posted by GitBox <gi...@apache.org>.
mattisonchao commented on code in PR #15942:
URL: https://github.com/apache/pulsar/pull/15942#discussion_r889927429
##########
pulsar-broker/src/main/java/org/apache/pulsar/broker/rest/TopicsBase.java:
##########
@@ -101,7 +117,11 @@
@Slf4j
public class TopicsBase extends PersistentTopicsBase {
+ private static final Map<String, Consumer<org.apache.pulsar.client.api.schema.GenericRecord>>
Review Comment:
We will pin the consumer in the specific broker After consumer-created, We will return the consumer broker URL, and the user must use this URL to do the next action.
So, After this, we will not affect by the topic moved.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] lhotari commented on pull request #15942: [feature][broker] Support HTTP endpoint consume messages
Posted by GitBox <gi...@apache.org>.
lhotari commented on PR #15942:
URL: https://github.com/apache/pulsar/pull/15942#issuecomment-1147120969
One reason for not adding this to Pulsar Brokers is that it will break Pulsar load balancing. @joefk Did a good write-up about this in https://lists.apache.org/thread/b7yjwn7cycp0r45mop4onfrr4mmp2tbd .
Quoting @joefk 's email:
> First, broker throughput and performance becomes unpredictable. The
current Pulsar load model (and it is used in the load manager for load
balancing) becomes unusable. Not only that, there will be no pre-computed
model that can be used in the load manager. Since the producer and
consumer randomly decide on what is the business logic,and the computation
can change based on the data, the model itself becomes dynamic and the
load manager has to rebuild the model anytime an user updates the business
logic. That is a tall order, worth years of work to implement.
The context was slightly different in that discussion, but I believe that it could be applied to this case too. We shouldn't be adding more business logic to brokers such as HTTP endpoint for consuming message. That could be implemented in a separate component which isn't part of the broker.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] mattisonchao commented on pull request #15942: [feature][broker] Support HTTP endpoint consume messages
Posted by GitBox <gi...@apache.org>.
mattisonchao commented on PR #15942:
URL: https://github.com/apache/pulsar/pull/15942#issuecomment-1147178378
> It might not be a good idea to add the implementation to the main Pulsar Admin API at all.
>
> HTTP consuming would be better to handle in a separate component. PIP-64 doesn't determine that this should be part of Pulsar Admin API and we should revisit this decision. I think it's a bad idea to add HTTP consuming to Pulsar Admin API and brokers.
>
> We need another discussion on the dev mailing list for deciding.
Yes, I also consider moving this part of the HTTP producer-consumer to pulsar-proxy, which will make the broker clear and reduce communication between brokers.
However, moving this part to the pulsar proxy will require the user to deploy another component, especially if the user is not running the pulsar in k8s.
I will draft a discussion in the dev mail list.
Best,
Mattison
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] merlimat commented on a diff in pull request #15942: [feature][broker] Support HTTP endpoint consume messages
Posted by GitBox <gi...@apache.org>.
merlimat commented on code in PR #15942:
URL: https://github.com/apache/pulsar/pull/15942#discussion_r890262298
##########
pulsar-broker/src/main/java/org/apache/pulsar/broker/rest/TopicsBase.java:
##########
@@ -101,7 +117,11 @@
@Slf4j
public class TopicsBase extends PersistentTopicsBase {
+ private static final Map<String, Consumer<org.apache.pulsar.client.api.schema.GenericRecord>>
Review Comment:
That is not a good model. It will create a very difficult API to use for the user. We need to do it such that it doesn’t matter which broker the consumer will hit.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] eolivelli commented on pull request #15942: [feature][broker] Support HTTP endpoint consume messages
Posted by GitBox <gi...@apache.org>.
eolivelli commented on PR #15942:
URL: https://github.com/apache/pulsar/pull/15942#issuecomment-1147291657
> Yes, I also consider moving this part of the HTTP producer-consumer to pulsar-proxy, which will make the broker clear and reduce communication between brokers.
you can build a "Proxy Extensions" and a "Protocol Handler", this way your code can run on the Proxy and on the Broker.
See how @cbornet did this for RabbitMQ:
- https://github.com/datastax/starlight-for-rabbitmq/blob/master/starlight-rabbitmq/src/main/java/com/datastax/oss/starlight/rabbitmq/GatewayProtocolHandler.java
- https://github.com/datastax/starlight-for-rabbitmq/blob/master/starlight-rabbitmq/src/main/java/com/datastax/oss/starlight/rabbitmq/GatewayProxyProtocolHandler.java
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] lhotari commented on a diff in pull request #15942: [feature][broker] Support HTTP endpoint consume messages
Posted by GitBox <gi...@apache.org>.
lhotari commented on code in PR #15942:
URL: https://github.com/apache/pulsar/pull/15942#discussion_r889874082
##########
pulsar-broker/src/main/java/org/apache/pulsar/broker/rest/TopicsBase.java:
##########
@@ -101,7 +117,11 @@
@Slf4j
public class TopicsBase extends PersistentTopicsBase {
+ private static final Map<String, Consumer<org.apache.pulsar.client.api.schema.GenericRecord>>
Review Comment:
How is the consumer lifecycle controlled? Does the consumer reside on just a single broker in a Pulsar cluster? Does a consumer get closed automatically when Pulsar load balancing moves the topic to another broker. Does a consumer expire?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] Stevenzall commented on pull request #15942: [feature][broker] Support HTTP endpoint consume messages
Posted by GitBox <gi...@apache.org>.
Stevenzall commented on PR #15942:
URL: https://github.com/apache/pulsar/pull/15942#issuecomment-1375619533
@mattisonchao What's the current progress？Could share the link?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] larshp commented on pull request #15942: [feature][broker] Support HTTP endpoint consume messages
Posted by "larshp (via GitHub)" <gi...@apache.org>.
larshp commented on PR #15942:
URL: https://github.com/apache/pulsar/pull/15942#issuecomment-1410501555
I found https://github.com/apache/pulsar/issues/2467 but it also does not keep track of the status on PIP-64
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org