Lari, I think there is a big miss. This issue is not _just_ for admin REST
API calls. It exists for every produce/consume client as well as in most
cases, the lookup service being used is the HTTPLookupService leading to
the same issue as you are mentioning for admin API calls. I've left a
comment for the same here -
https://github.com/apache/pulsar/pull/22039#discussion_r1487332735

On Tue, Feb 13, 2024 at 12:53 PM Lari Hotari <lhot...@apache.org> wrote:

> On 2024/02/13 06:46:29 Girish Sharma wrote:
> > Personally, while this may be a much cleaner approach or may be not solve
> > the core issue at all, it is not what we are trying to achieve with our
> > PIP, which is basically only a PIP due to configuration changes involved,
> > but actually is a bug fix for a *bug we are facing in production right
> now.
> > *The same bug is also highlighted by you via the comment you have linked
> in
> > your [3] link
> > There are much more things to consider for the multiple bind address
> > approach and it deserves its own PIP. Specifically the comment on GH that
> > I've made showcasing a use case -
> > https://github.com/apache/pulsar/pull/22039#discussion_r1486400014
>
> The multiple bind address solution is already implemented for the Pulsar
> binary protocol as it is defined in PIP-95. What we are dealing with a gap
> in the solution, mainly about Pulsar Admin API http/https redirects.
> I replied in
> https://github.com/apache/pulsar/pull/22039#discussion_r1487282934 how
> this could possibly be achieved without too many changes.
> That's not a complete design since there would have to be a way to set the
> header value and documentation about how this should be achieved. One
> possibility is that there's a proxy server for the external address which
> sets this header value. (PIP-95 points into that direction)
>
> > Let's treat this PIP as a bug fix only. If needed, we can skip the PIP
> and
> > directly send a bug fix PR if that clears things here.
>
> I'm not sure if that's optimal. A PIP is usually issued when a public
> interface is changed. I agree that this is addressing gaps left in PIP-61 &
> PIP-95 and could be considered as an extension. However it's easier to add
> a new PIP than start extending old PIPs. The benefit of the PIP process is
> that it helps getting consensus about the design before someone puts a lot
> of effort in implementing something that would not be accepted.
> Since we don't have consensus, I think it's better to continue the
> discussion and also meet at the Pulsar community meeting to discuss this
> further. Hopefully others from the community also participate.
>
> > I disagree here. We have clearly identified that there is a bug in the
> > current code. We are trying to do a bug fix here. The goal is not to deep
>
> I'm not sure if it's a bug. The use case isn't covered by PIP-61/PIP-95.
> That is a gap in the design to me, not an implementation bug. However it
> doesn't really matter in the end what we call it. The end-to-end
> functionality isn't usable at the moment for all Pulsar Admin API calls.
>
> > dive into a much bigger design problem as we want to hotfix this ASAP in
> > our system, but also want an alignment with the community so as to not
> > maintain this patch locally, internally, for every version we upgrade to.
>
> Yes, that is a sensible goal. I'm pretty sure that we can achieve a
> solution that addresses the main gap in PIP-61&PIP-95 which is about the
> Pulsar Admin API redirects. It is also notable that there are gaps in the
> documentation of the advertised listeners in Pulsar [1]. The documentation
> also needs some more love. Contributions are more than welcome!
>
> -Lari
>
> 1 -
> https://pulsar.apache.org/docs/next/concepts-multiple-advertised-listeners/#advertised-listeners
>


-- 
Girish Sharma

Reply via email to