[GitHub] [pulsar-helm-chart] vitosans commented on issue #84: TLS Authentication in Kubernetes, Pulsar 2.6.1 - Broker crash loop on startup due to 401 in WorkerService.start(..)
vitosans commented on issue #84: URL: https://github.com/apache/pulsar-helm-chart/issues/84#issuecomment-932431670 I did something like this: tlsEnabled: "true" brokerClientTlsEnabled: "true" brokerClientTrustCertsFilePath: "/pulsar/certs/ca/ca.crt" useTls: true tlsCertificateFilePath: "/pulsar/certs/broker/tls.crt" tlsKeyFilePath: "/pulsar/certs/broker/tls.key" tlsTrustCertsFilePath: "/pulsar/certs/ca/ca.crt" tlsAllowInsecureConnection: false tlsEnableHostnameVerification: false tlsCertRefreshCheckDurationSec: 300 In broker-configmap.yaml The broker is now able to start up when functions are enabled. Now the problem is when you deploy a function the functions_worker that gets spawned off has a default functions_works.yaml and not the one generated from bin/gen-yml-from-env.py conf/functions_worker.yml in the StateFullSet So of course he now gets a: HTTP 401 Unauthorized │ Reason: HTTP 401 Unauthorized as he is trying to post to http://localhost:8080 which of course is wrong :) Trying to debug this currently, and then make a giant PR that enables mTLS -- 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-helm-chart] vitosans edited a comment on issue #84: TLS Authentication in Kubernetes, Pulsar 2.6.1 - Broker crash loop on startup due to 401 in WorkerService.start(..)
vitosans edited a comment on issue #84:
URL:
https://github.com/apache/pulsar-helm-chart/issues/84#issuecomment-932431670
I did something like this:
```
webServiceUrl: "https://{{ template "pulsar.fullname" . }}-{{
.Values.proxy.component }}:{{ .Values.proxy.ports.https }}/"
brokerServiceUrl: "pulsar+ssl://{{ template "pulsar.fullname" . }}-{{
.Values.proxy.component }}:{{ .Values.proxy.ports.pulsarssl }}/"
tlsEnabled: "true"
brokerClientTlsEnabled: "true"
brokerClientTrustCertsFilePath: "/pulsar/certs/ca/ca.crt"
useTls: true
tlsCertificateFilePath: "/pulsar/certs/broker/tls.crt"
tlsKeyFilePath: "/pulsar/certs/broker/tls.key"
tlsTrustCertsFilePath: "/pulsar/certs/ca/ca.crt"
tlsAllowInsecureConnection: false
tlsEnableHostnameVerification: false
tlsCertRefreshCheckDurationSec: 300
```
In broker-configmap.yaml
The broker is now able to start up when functions are enabled. Now the
problem is when you deploy a function the functions_worker that gets spawned
off has a default functions_works.yaml and not the one generated from
bin/gen-yml-from-env.py conf/functions_worker.yml in the StateFullSet
So of course he now gets a:
HTTP 401 Unauthorized
│
Reason: HTTP 401 Unauthorized
as he is trying to post to http://localhost:8080 which of course is wrong :)
Trying to debug this currently, and then make a giant PR that enables mTLS
--
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: Separate topic and partition label for Prometheus metrics in 2.9.0
> I believe that a middle ground option is to set it to 'true' in 2.9. > It is a burden to maintain so many options for us, so I agree that we can > drop it for 2.10. +1, I think Enrico's idea sounds like a good way to ensure a gradual change. By keeping the config in 2.9, we can add a note in the `broker.conf`, the `standalone.conf`, and the `ServiceConfiguration.java` files to let users know know the config is deprecated and will be removed in 2.10. We can also explain the upgrade path in the release notes to help users update their monitoring stack (like grafana graphs) accordingly. Thanks, Michael On Wed, Sep 29, 2021 at 11:46 PM Enrico Olivelli wrote: > > Penghui, > > Il Gio 30 Set 2021, 06:10 PengHui Li ha scritto: > > > Hi all, > > > > Hope you are well, currently, we are exposing the topic metrics with an > > open one label topic which > > is the complete topic name such as > > `persistent://public/default/xxx-partition-0`, but as > > https://github.com/apache/pulsar/issues/11432 mentioned, we are not able > > to > > have an aggregated metrics for the partitioned topic, so PR > > https://github.com/apache/pulsar/pull/12225 > > will improve this case which can allow users to expose 2 labels, topic, and > > partition. > > > > But in order not to break compatibility, I have added an option ` > > splitTopicAndPartitionLabelInPrometheus` to enable the separate partition > > label and by default, it should be false. So it's a 2 step upgrade story, > > upgrade the broker, and enable the option. > > > > I want to start a discussion for can we remove the option in 2.9? we have > > many options and for the new major version, we do not want users to have an > > option for the single label for 2 labels. Since we should ship the > > enhancement in branch-2.8, so I have merged the PR which I can cherry-pick > > to branch-2.8. > > > > If we agree to remove the option in 2.9.0, I can push a PR to remove the > > option in the master branch. > > > > I believe that a middle ground option is to set it to 'true' in 2.9. > > There may be users that need to adapt their monitoring systems, so without > a configuration option they will have to both upgrade Pulsar and their > systems in one shot. > > It is a burden to maintain so many options for us, so I agree that we can > drop it for 2.10. > > So my proposal is to not drop it but to set it to 'true' for 2.9. > > Thanks > Enrico > > > > > Thanks, > > Penghui > >
Re: [VOTE] PIP-96 Message payload processor for Pulsar client
+1 binding On Thu, Sep 30, 2021 at 2:06 AM PengHui Li wrote: > +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 > > >
Re: [VOTE] PIP-97 Asynchronous Authentication Provider
+1 binding On Thu, Sep 30, 2021 at 10:54 AM Matteo Merli wrote: > 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
I am with Enrrico on this, and for more or less the same reasons.
My vote is to update the doc to say that on close(), there are no
guarantees on pending messages.
There is no reasonable basis for any user to claim that messages are not
getting flushed, because there are no guarantees that the flush will
result in successful publishing. User expectations and reality are
different, even with today's implementation.
if someone really requires a flush within close(), that's already
possible by doing a flush before the close.
The best thing to do is to update the close() documentation. (1) Remove
mention of flushing from the javadoc, and (2) add that close does not
guarantee that outstanding messages are published successfully.
On Thu, Sep 30, 2021 at 9:52 PM Michael Marshall
wrote:
> 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 <
> mikemars...@gmail.com> 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` interf
Re: [VOTE] PIP-96 Message payload processor for Pulsar client
+1 (binding) Thanks, Hang Sijie Guo 于2021年10月2日周六 上午5:11写道： > > +1 binding > > On Thu, Sep 30, 2021 at 2:06 AM PengHui Li wrote: > > > +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 > > > > >
