[GitHub] [pulsar-dotpulsar] PetterIsberg closed pull request #85: Add a watchdog to the connections
PetterIsberg closed pull request #85: URL: https://github.com/apache/pulsar-dotpulsar/pull/85 -- 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: dev-unsubscr...@pulsar.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [pulsar-dotpulsar] PetterIsberg commented on issue #84: Recover after network disconnect
PetterIsberg commented on issue #84: URL: https://github.com/apache/pulsar-dotpulsar/issues/84#issuecomment-931009908 I also think adding pings to the client is a good idea. I closed my PR as it was meant as an example of how it could be solved. I should probably have marked it WIP. -- 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: dev-unsubscr...@pulsar.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [VOTE] PIP-96 Message payload processor for Pulsar client
+1 (binding) Thanks for the great work. Penghui On Thu, Sep 30, 2021 at 12:38 PM Enrico Olivelli wrote: > +1 (binding) > > Enrico > > Il Gio 30 Set 2021, 06:08 Yunze Xu ha > scritto: > > > Hi folks, > > > > It has been about two weeks since I opened the PIP-96 issue and the > design > > has > > changed a lot. Thanks a lot for @eolivelli's suggestions. I think now > it's > > time > > to start a vote. > > > > PIP-96 issue: https://github.com/apache/pulsar/issues/12087 < > > https://github.com/apache/pulsar/issues/12087> > > > > Thanks, > > Yunze >
[GitHub] [pulsar-client-node] arihantdaga commented on issue #161: Looks like the pulsar-client-node can not run with the current pulsar master code
arihantdaga commented on issue #161: URL: https://github.com/apache/pulsar-client-node/issues/161#issuecomment-931241256 I am facing the same. I installed pulsar cpp client in ubuntu following the instructions on https://pulsar.apache.org/docs/en/client-libraries-cpp/, using the deb package, it has also created libpulsar and libpulsar.so in my /usr/lib folder. But i don't know where these files should be found - '' -- 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: dev-unsubscr...@pulsar.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [pulsar-dotpulsar] dunkymole commented on issue #84: Recover after network disconnect
dunkymole commented on issue #84: URL: https://github.com/apache/pulsar-dotpulsar/issues/84#issuecomment-931340162 OK great. Agree sending pings is a much better way to detect the half-broken status. I wasnt sure whether it would be feasible with the wire-protocol, I never checked. Thanks @blankensteiner, looking forward to that release :-) -- 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: dev-unsubscr...@pulsar.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: Correct semantics of producer close
I have two questions: 1. Does close imply immediate shutdown? 2. Does close imply flush? There is not yet consensus on 1, and 2 is only relevant if 1's answer is "no". Thus far, the conversation has centered on the `Producer#close` method. I'd like to broaden the discussion to include some other methods from the `PulsarClient` interface: `shutdown` and `close`. The Javadoc for `PulsarClient#shutdown` describes the "shutdown immediately" behavior. It says: > Release all the resources and close all the producer, consumer and > reader instances without waiting for ongoing operations to complete. The Javadoc for `PulsarClient#close` describes waiting for pending/in-flight messages to complete before returning. It says: > This operation will trigger a graceful close of all producer, consumer > and reader instances that this client has currently active. That implies > that close will block and wait until all pending producer send requests > are persisted. One question that follows from the above: why does the `Producer` not have a `shutdown` method? I think this is because the "immediate shutdown" behavior is not necessary for a single producer. When immediate shutdown semantics are required, the `PulsarClient#shutdown` method is sufficient because it is used when shutting down the whole application. (If this is not correct, perhaps we should add a `shutdown` method to the producer?) Since immediate shutdown semantics are already available via our client API, I posit that the answer to question 1 is no, `close` does not imply immediate shutdown. At the very least, `close` in the Pulsar Client has not historically implied immediate shutdown. Additionally, it is relevant to point out that the `Producer#close` method is already sending a `CLOSE_PRODUCER` command and waiting on a response back from the broker. The broker's producer close method has the following Javadoc: > Close the producer immediately if: a. the connection is dropped > b. it's a graceful close and no pending publish acks are left > else wait for pending publish acks Since we're already waiting on the broker to respond to the producer's `CLOSE_PRODUCER` request, I see no reason to fail pending/in-flight messages immediately, especially because we should get a response back for those messages before getting the `SUCCESS` response from the broker since the responses will come on the same TCP connection. We could even simplify the close logic so that when the `CLOSE_PRODUCER` request completes (either successfully or because of a failure), we fail all remaining pending message futures. Ultimately, we need to decide whether to update the implementation to match the existing Javadocs, or to update the Javadocs to indicate that `close` means an immediate shutdown, which includes failing all outstanding message futures immediately. My vote is to make the implementation align with the Javadocs. Regarding question 2, I prefer that `close` implies flush because it is only a single (batched) message being flushed. If we do flush this message, we'll need to make sure that the message is sent before the `CLOSE_PRODUCER` command is sent. Thanks, Michael On Wed, Sep 29, 2021 at 7:04 AM Enrico Olivelli wrote: > > I agree that we must ensure that every pending callback must be completed > eventually (timeout or another error is not a problem), > because we cannot let the client application hang forever. > I believe that the application can perform a flush() explicitly and also > wait for every callback to be executed if that is the requirement. > > Usually you call close() when: > 1. you have a serious problem: you already know that there is a hard error, > and you want to close the Producer or the Application and possibly start a > new one to recover > 2. you are shutting down your application or component: you have control > over the callbacks, so you can wait for them to complete > > So case 2. can be covered by the application. We have to support case 1: > fail fast and close (no need for flush()) . > > In my experience trying to implement "graceful stops" adds only complexity > and false hopes to the users. > > Enrico > > > > Il giorno mer 29 set 2021 alle ore 13:58 Nirvana <1572139...@qq.com.invalid> > ha scritto: > > > I agree to try to ensure ”at most once“ when closing。 > > > > > > > That would still get controlled by send timeout, after that, the send > > will fail and close should proceed. > > This sounds more in line with “at most once”。 > > > > > > -- 原始邮件 -- > > 发件人: > > "dev" > > < > > matteo.me...@gmail.com>; > > 发送时间: 2021年9月29日(星期三) 下午3:55 > > 收件人: "Dev" > > > 主题: Re: Correct semantics of producer close > > > > > > > > > but equally they might be > > > surprised when closeAsync doesn't complete because the pending > > messages > > > can't be cleared > > > > That would still get controll
Re: [VOTE] PIP-97 Asynchronous Authentication Provider
LGTM +1 -- Matteo Merli On Mon, Sep 27, 2021 at 1:43 PM Michael Marshall wrote: > > Hi Pulsar Community, > > I would like to start a VOTE for PIP-97 Asynchronous Authentication Provider. > > The issue for this PIP is here: > https://github.com/apache/pulsar/issues/12105. > > The PR for all interface changes this PIP requires are here: > https://github.com/apache/pulsar/pull/12104. Note that I deprecate > several authentication methods that this PIP renders unnecessary. > > Please VOTE within 72 hours. > > Thanks, > Michael
Re: [VOTE] PIP-97 Asynchronous Authentication Provider
+1 (binding) Enrico Il Gio 30 Set 2021, 19:54 Matteo Merli ha scritto: > LGTM +1 > > > -- > Matteo Merli > > > On Mon, Sep 27, 2021 at 1:43 PM Michael Marshall > wrote: > > > > Hi Pulsar Community, > > > > I would like to start a VOTE for PIP-97 Asynchronous Authentication > Provider. > > > > The issue for this PIP is here: > > https://github.com/apache/pulsar/issues/12105. > > > > The PR for all interface changes this PIP requires are here: > > https://github.com/apache/pulsar/pull/12104. Note that I deprecate > > several authentication methods that this PIP renders unnecessary. > > > > Please VOTE within 72 hours. > > > > Thanks, > > Michael >
Re: Correct semantics of producer close
Following up here. I am pretty sure part of this conversation has been based on a misunderstanding of the code. From what I can tell, the behavior for `Producer#closeAsync` in the client (mostly) aligns with the current Javadocs. > The existing implementation of producer close is: > 1. Cancel all timers, including send and batch container > (`batchMessageContainer`). > 2. Complete all pending messages (`pendingMessages`) with > `AlreadyCloseException`. I agree with 1, but I think 2 is only partially correct. The client will only exceptionally complete pending messages if the connection is null or not ready, or after the broker responds to the `CLOSE_PRODUCER` command or a timeout passes. This behavior seems right to me. Here is the relevant code: https://github.com/apache/pulsar/blob/9d309145f342bc416b8b4663125e1216903a3e83/pulsar-client/src/main/java/org/apache/pulsar/client/impl/ProducerImpl.java#L875-L909 The only remaining question: does close imply flush? If not, we'll need update the logic to fail the messages contained in the `batchMessageContainer` during close. Otherwise, we'll update the logic to call flush before sending the `CLOSE_PRODUCER` command and everything should work as documented. In both cases, we should update the Javadocs to make the behavior clearer. Thanks, Michael On Thu, Sep 30, 2021 at 11:55 AM Michael Marshall wrote: > > I have two questions: > > 1. Does close imply immediate shutdown? > 2. Does close imply flush? > > There is not yet consensus on 1, and 2 is only relevant if 1's answer is "no". > > Thus far, the conversation has centered on the `Producer#close` > method. I'd like to broaden the discussion to include some other > methods from the `PulsarClient` interface: `shutdown` and `close`. > > The Javadoc for `PulsarClient#shutdown` describes the "shutdown > immediately" behavior. It says: > > > Release all the resources and close all the producer, consumer and > > reader instances without waiting for ongoing operations to complete. > > The Javadoc for `PulsarClient#close` describes waiting for > pending/in-flight messages to complete before returning. It says: > > > This operation will trigger a graceful close of all producer, consumer > > and reader instances that this client has currently active. That implies > > that close will block and wait until all pending producer send requests > > are persisted. > > One question that follows from the above: why does the `Producer` not > have a `shutdown` method? I think this is because the "immediate > shutdown" behavior is not necessary for a single producer. When > immediate shutdown semantics are required, the `PulsarClient#shutdown` > method is sufficient because it is used when shutting down the whole > application. (If this is not correct, perhaps we should add a > `shutdown` method to the producer?) > > Since immediate shutdown semantics are already available via our > client API, I posit that the answer to question 1 is no, `close` does > not imply immediate shutdown. At the very least, `close` in the Pulsar > Client has not historically implied immediate shutdown. > > Additionally, it is relevant to point out that the `Producer#close` > method is already sending a `CLOSE_PRODUCER` command and waiting on a > response back from the broker. The broker's producer close method has > the following Javadoc: > > > Close the producer immediately if: a. the connection is dropped > > b. it's a graceful close and no pending publish acks are left > > else wait for pending publish acks > > Since we're already waiting on the broker to respond to the producer's > `CLOSE_PRODUCER` request, I see no reason to fail pending/in-flight > messages immediately, especially because we should get a response back > for those messages before getting the `SUCCESS` response from the > broker since the responses will come on the same TCP connection. We > could even simplify the close logic so that when the `CLOSE_PRODUCER` > request completes (either successfully or because of a failure), we > fail all remaining pending message futures. > > Ultimately, we need to decide whether to update the implementation to > match the existing Javadocs, or to update the Javadocs to indicate > that `close` means an immediate shutdown, which includes failing all > outstanding message futures immediately. My vote is to make the > implementation align with the Javadocs. > > Regarding question 2, I prefer that `close` implies flush because it > is only a single (batched) message being flushed. If we do flush this > message, we'll need to make sure that the message is sent before > the `CLOSE_PRODUCER` command is sent. > > Thanks, > Michael > > > On Wed, Sep 29, 2021 at 7:04 AM Enrico Olivelli wrote: > > > > I agree that we must ensure that every pending callback must be completed > > eventually (timeout or another error is not a problem), > > because we cannot let the client application hang forever. > > I believe that the application can perform a flush()
[GitHub] [pulsar-dotpulsar] blankensteiner commented on issue #84: Recover after network disconnect
blankensteiner commented on issue #84: URL: https://github.com/apache/pulsar-dotpulsar/issues/84#issuecomment-931642288 Hi @PetterIsberg, @Flipbed, and @dunkymole. Thanks for the heads up on this! :-) Since I can't reproduce the problem locally I'm hoping you might help me test the bites 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: dev-unsubscr...@pulsar.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: Correct semantics of producer close
You're right that before a CommandCloseProducer request was completed, the
pending messages should be persisted before this close request was completed
in normal cases. It’s guaranteed by broker side.
Then there’s no inconsistency between the implementation and JavaDocs now.
The key point is whether should we flush the messages in batch container. I
prefer
keeping the current semantics. But I found the messages in batch container
never failed. We need to fix the problem. For example, here’s my unit test:
```java
@Test
public void test() throws Exception {
final Producer producer =
pulsarClient.newProducer(Schema.STRING)
.topic("topic")
.batchingMaxMessages(1)
.batchingMaxBytes(1000)
.batchingMaxPublishDelay(100, TimeUnit.SECONDS)
.sendTimeout(1, TimeUnit.SECONDS)
.create();
final CountDownLatch latch = new CountDownLatch(10);
final Map throwableMap = new ConcurrentHashMap<>();
for (int i = 0; i < 10; i++) {
final Integer index = i;
producer.sendAsync("msg-" + i).whenComplete((id, e) -> {
if (e != null) {
throwableMap.put(index, e);
}
latch.countDown();
});
}
producer.close();
latch.await();
throwableMap.forEach((i, e) -> {
log.info("Message {} failed with {}", i, e);
});
}
```
The test would block forever.
> 2021年10月1日 上午4:22，Michael Marshall 写道：
>
> Following up here. I am pretty sure part of this conversation has been
> based on a misunderstanding of the code. From what I can tell, the
> behavior for `Producer#closeAsync` in the client (mostly) aligns with
> the current Javadocs.
>
>> The existing implementation of producer close is:
>> 1. Cancel all timers, including send and batch container
>> (`batchMessageContainer`).
>> 2. Complete all pending messages (`pendingMessages`) with
>> `AlreadyCloseException`.
>
> I agree with 1, but I think 2 is only partially correct. The client
> will only exceptionally complete pending messages if the connection is
> null or not ready, or after the broker responds to the
> `CLOSE_PRODUCER` command or a timeout passes. This behavior seems
> right to me.
>
> Here is the relevant code:
> https://github.com/apache/pulsar/blob/9d309145f342bc416b8b4663125e1216903a3e83/pulsar-client/src/main/java/org/apache/pulsar/client/impl/ProducerImpl.java#L875-L909
>
> The only remaining question: does close imply flush? If not, we'll
> need update the logic to fail the messages contained in the
> `batchMessageContainer` during close. Otherwise, we'll update the
> logic to call flush before sending the `CLOSE_PRODUCER` command and
> everything should work as documented. In both cases, we should update
> the Javadocs to make the behavior clearer.
>
> Thanks,
> Michael
>
>
>
> On Thu, Sep 30, 2021 at 11:55 AM Michael Marshall
> wrote:
>>
>> I have two questions:
>>
>> 1. Does close imply immediate shutdown?
>> 2. Does close imply flush?
>>
>> There is not yet consensus on 1, and 2 is only relevant if 1's answer is
>> "no".
>>
>> Thus far, the conversation has centered on the `Producer#close`
>> method. I'd like to broaden the discussion to include some other
>> methods from the `PulsarClient` interface: `shutdown` and `close`.
>>
>> The Javadoc for `PulsarClient#shutdown` describes the "shutdown
>> immediately" behavior. It says:
>>
>>> Release all the resources and close all the producer, consumer and
>>> reader instances without waiting for ongoing operations to complete.
>>
>> The Javadoc for `PulsarClient#close` describes waiting for
>> pending/in-flight messages to complete before returning. It says:
>>
>>> This operation will trigger a graceful close of all producer, consumer
>>> and reader instances that this client has currently active. That implies
>>> that close will block and wait until all pending producer send requests
>>> are persisted.
>>
>> One question that follows from the above: why does the `Producer` not
>> have a `shutdown` method? I think this is because the "immediate
>> shutdown" behavior is not necessary for a single producer. When
>> immediate shutdown semantics are required, the `PulsarClient#shutdown`
>> method is sufficient because it is used when shutting down the whole
>> application. (If this is not correct, perhaps we should add a
>> `shutdown` method to the producer?)
>>
>> Since immediate shutdown semantics are already available via our
>> client API, I posit that the answer to question 1 is no, `close` does
>> not imply immediate shutdown. At the very least, `close` in the Pulsar
>> Client has not historically implied immediate shutdown.
>>
>> Additionally, it is relevant to point out that the `Producer#close`
>> method is already sending a `CLOSE_PRODUCER` command and waiting on a
>> response back from the broker. The br
Re: Correct semantics of producer close
Thanks for your analysis, Yunze. I identified above that the messages
in the batch container were not getting completed correctly, so I put
together a PR to fix the problematic behavior. This PR will be valid
regardless of our decision to add flush logic to the close method.
Here is the PR: https://github.com/apache/pulsar/pull/12259.
Thanks,
Michael
On Thu, Sep 30, 2021 at 10:27 PM Yunze Xu wrote:
>
> You're right that before a CommandCloseProducer request was completed, the
> pending messages should be persisted before this close request was completed
> in normal cases. It’s guaranteed by broker side.
>
> Then there’s no inconsistency between the implementation and JavaDocs now.
>
> The key point is whether should we flush the messages in batch container. I
> prefer
> keeping the current semantics. But I found the messages in batch container
> never failed. We need to fix the problem. For example, here’s my unit test:
>
> ```java
> @Test
> public void test() throws Exception {
> final Producer producer =
> pulsarClient.newProducer(Schema.STRING)
> .topic("topic")
> .batchingMaxMessages(1)
> .batchingMaxBytes(1000)
> .batchingMaxPublishDelay(100, TimeUnit.SECONDS)
> .sendTimeout(1, TimeUnit.SECONDS)
> .create();
> final CountDownLatch latch = new CountDownLatch(10);
> final Map throwableMap = new
> ConcurrentHashMap<>();
> for (int i = 0; i < 10; i++) {
> final Integer index = i;
> producer.sendAsync("msg-" + i).whenComplete((id, e) -> {
> if (e != null) {
> throwableMap.put(index, e);
> }
> latch.countDown();
> });
> }
> producer.close();
> latch.await();
> throwableMap.forEach((i, e) -> {
> log.info("Message {} failed with {}", i, e);
> });
> }
> ```
>
> The test would block forever.
>
> > 2021年10月1日 上午4:22，Michael Marshall 写道：
> >
> > Following up here. I am pretty sure part of this conversation has been
> > based on a misunderstanding of the code. From what I can tell, the
> > behavior for `Producer#closeAsync` in the client (mostly) aligns with
> > the current Javadocs.
> >
> >> The existing implementation of producer close is:
> >> 1. Cancel all timers, including send and batch container
> >> (`batchMessageContainer`).
> >> 2. Complete all pending messages (`pendingMessages`) with
> >> `AlreadyCloseException`.
> >
> > I agree with 1, but I think 2 is only partially correct. The client
> > will only exceptionally complete pending messages if the connection is
> > null or not ready, or after the broker responds to the
> > `CLOSE_PRODUCER` command or a timeout passes. This behavior seems
> > right to me.
> >
> > Here is the relevant code:
> > https://github.com/apache/pulsar/blob/9d309145f342bc416b8b4663125e1216903a3e83/pulsar-client/src/main/java/org/apache/pulsar/client/impl/ProducerImpl.java#L875-L909
> >
> > The only remaining question: does close imply flush? If not, we'll
> > need update the logic to fail the messages contained in the
> > `batchMessageContainer` during close. Otherwise, we'll update the
> > logic to call flush before sending the `CLOSE_PRODUCER` command and
> > everything should work as documented. In both cases, we should update
> > the Javadocs to make the behavior clearer.
> >
> > Thanks,
> > Michael
> >
> >
> >
> > On Thu, Sep 30, 2021 at 11:55 AM Michael Marshall
> > wrote:
> >>
> >> I have two questions:
> >>
> >> 1. Does close imply immediate shutdown?
> >> 2. Does close imply flush?
> >>
> >> There is not yet consensus on 1, and 2 is only relevant if 1's answer is
> >> "no".
> >>
> >> Thus far, the conversation has centered on the `Producer#close`
> >> method. I'd like to broaden the discussion to include some other
> >> methods from the `PulsarClient` interface: `shutdown` and `close`.
> >>
> >> The Javadoc for `PulsarClient#shutdown` describes the "shutdown
> >> immediately" behavior. It says:
> >>
> >>> Release all the resources and close all the producer, consumer and
> >>> reader instances without waiting for ongoing operations to complete.
> >>
> >> The Javadoc for `PulsarClient#close` describes waiting for
> >> pending/in-flight messages to complete before returning. It says:
> >>
> >>> This operation will trigger a graceful close of all producer, consumer
> >>> and reader instances that this client has currently active. That implies
> >>> that close will block and wait until all pending producer send requests
> >>> are persisted.
> >>
> >> One question that follows from the above: why does the `Producer` not
> >> have a `shutdown` method? I think this is because the "immediate
> >> shutdown" behavior is not necessary for a single producer. When
> >> immediate shutdown semantics are required, the `PulsarClient#shutdown`
> >> method is sufficient because
