Gwen, Ashish,
Rolling upgrade should be fine in this case. Users can
bring down single broker at a time and upgrade their
kafka binaries along with new authorizer
implementation . This looks fine to me.
Thanks,
Harsha
On Thu, Apr 7, 2016, at 03:15 PM, Ashish Singh wrote:
> Harsha,
>
> No, ZK data format for acls or SimplaAclAthorizer implementation is not
> proposed to be changed.
>
> Gwen,
>
> The concern I was pointing at is that if we choose to go with Option 2,
> i.e., move interface to a separate package, authorizer implementation
> will
> have to be updated along with broker upgrade and authorizer
> implementations
> need to make changes. I am not sure how this affects rolling upgrade.
> Harsha, do you mind sharing your concerns on rolling upgrade?
>
> On Thu, Apr 7, 2016 at 12:52 PM, Harsha <ka...@harsha.io> wrote:
>
> > Ashish,
> > Thanks for the details. We are not changing any of the zookeeper
> > data format for acls right?
> >
> > Thanks,
> > Harsha
> >
> > On Thu, Apr 7, 2016, at 11:25 AM, Gwen Shapira wrote:
> > > Can you guys go into details on what will happen during a rolling upgrade
> > > exactly?
> > >
> > > Gwen
> > >
> > > On Thu, Apr 7, 2016 at 11:21 AM, Ashish Singh <asi...@cloudera.com>
> > > wrote:
> > >
> > > > Hello Harsha,
> > > >
> > > > On Thu, Apr 7, 2016 at 11:03 AM, Harsha <m...@harsha.io> wrote:
> > > >
> > > > "My only ask is to have this in 0.10. As Jay pointed out, right now
> > > > > there
> > > > > are not many implementations out there, we might want to fix it
> > ASAP."
> > > > >
> > > > > Probably there aren't many implementations but there are lot of users
> > > > > using these implementations in production clusters.
> > > > > Isn't this going to break the rolling upgrade?
> > > >
> > > > It will and it is a concern, in my previous mail I have mentioned this
> > as
> > > > an issue if we choose to go this route. However, if we actually decide
> > to
> > > > do this, I would say it is better to do it sooner than later, as fewer
> > > > implementations will be affected. Below is excerpt from my previous
> > mail.
> > > >
> > > > Increase scope of KIP-50 to move authorizer and related classes to a
> > > > separate package. The new package will have java interface. This will
> > allow
> > > > implementations to not depend on kafka core and just on authorizer
> > package,
> > > > make authorization interface follow kafka’s coding standards and will
> > allow
> > > > java implementations to be cleaner. We can either completely drop scala
> > > > interface, which might be a pain for existing implementations, or we
> > can
> > > > have scala interface wrap java interface. Later allows a cleaner
> > > > deprecation path for existing scala authorizer interface, however it
> > may or
> > > > may not be feasible as Kafka server will have to somehow decide which
> > > > interface it should be looking for while loading authorizer
> > implementation,
> > > > this can probably be solved with a config or some reflection. If we
> > choose
> > > > to go this route, I can dig deeper.
> > > >
> > > > If we go with option 2 and commit on getting this in ASAP, preferably
> > in
> > > > 0.10, there will be fewer implementations that will be affected.
> > > >
> > > > and also moving to Java ,
> > > > > a authorizer implementation going to run inside a KafkaBroker and I
> > > > > don't see why this is necessary to move to clients package.
> > > > > Are we planning on introducing common module to have it independent
> > of
> > > > > broker and client code?
> > > > >
> > > > Yes, I think that would take away the requirement of depending on Kafka
> > > > core from authorizer implementations. Do you suggest otherwise?
> > > >
> > > >
> > > > > -Harsha
> > > > >
> > > > > On Thu, Apr 7, 2016, at 10:52 AM, Ashish Singh wrote:
> > > > > > We might want to take a call here. Following are the options.
> > > > > >
> > > > > > 1. Let KIP-50 be the way it is, i.e., just add getDescription()
> > to
> > > > > > existing scala authorizer interface. It will break binary
> > > > > > compatibility
> > > > > > (only when using CLI and/or AdminCommand from >= 0.10 against
> > > > > > authorizer
> > > > > > implementations based on 0.9.). If we go this route, it is a
> > simpler
> > > > > > change
> > > > > > and existing implementations won’t have to change anything on
> > their
> > > > > > end.
> > > > > > 2. Increase scope of KIP-50 to move authorizer and related
> > classes
> > > > to
> > > > > > a
> > > > > > separate package. The new package will have java interface. This
> > > > will
> > > > > > allow
> > > > > > implementations to not depend on kafka core and just on
> > authorizer
> > > > > > package,
> > > > > > make authorization interface follow kafka’s coding standards and
> > > > will
> > > > > > allow
> > > > > > java implementations to be cleaner. We can either completely
> > drop
> > > > > > scala
> > > > > > interface, which might be a pain for existing implementations,
> > or we
> > > > > > can
> > > > > > have scala interface wrap java interface. Later allows a cleaner
> > > > > > deprecation path for existing scala authorizer interface,
> > however it
> > > > > > may or
> > > > > > may not be feasible as Kafka server will have to somehow decide
> > > > which
> > > > > > interface it should be looking for while loading authorizer
> > > > > > implementation,
> > > > > > this can probably be solved with a config or some reflection.
> > If we
> > > > > > choose
> > > > > > to go this route, I can dig deeper.
> > > > > >
> > > > > > If we decide to go with option 1, I think it would be fair to say
> > that
> > > > > > scala authorizer interface will be around for some time, as there
> > will
> > > > be
> > > > > > more implementations relying on it. If we go with option 2 and
> > commit
> > > > on
> > > > > > getting this in ASAP, preferably in 0.10, there will be fewer
> > > > > > implementations that will be affected.
> > > > > >
> > > > > > *Another thing to notice is that scala authorizer interface is not
> > > > > > annotated as unstable.*
> > > > > >
> > > > > >
> > > > > > On Wed, Apr 6, 2016 at 9:41 AM, Ashish Singh <asi...@cloudera.com>
> > > > > wrote:
> > > > > >
> > > > > > > I see value in minimizing breaking changes and I do not oppose
> > the
> > > > > idea of
> > > > > > > increasing scope of KIP-50 to move auth interface to java.
> > > > > > >
> > > > > > > As authorizer implementations do not really need to depend on
> > Kafka
> > > > > core,
> > > > > > > I would suggest that we keep authorizer interface and its
> > components
> > > > > in a
> > > > > > > separate package. I share the concern that right now using
> > Resource,
> > > > > > > Operation, etc, in java implementations is messy. I had to deal
> > with
> > > > > lot of
> > > > > > > it while writing Apache Sentry plugin.
> > > > > > >
> > > > > > > My only ask is to have this in 0.10. As Jay pointed out, right
> > now
> > > > > there
> > > > > > > are not many implementations out there, we might want to fix it
> > ASAP.
> > > > > I can
> > > > > > > only speak of Sentry integration and I think 0.10 will be the
> > best
> > > > for
> > > > > such
> > > > > > > a change, as I should be able to adopt the changes in Sentry
> > > > > integration
> > > > > > > before a lot of users start using it.
> > > > > > >
> > > > > > > On Wed, Apr 6, 2016 at 9:25 AM, Ismael Juma <ism...@juma.me.uk>
> > > > wrote:
> > > > > > >
> > > > > > >> It is small, but breaks binary compatibility.
> > > > > > >>
> > > > > > >> Ismael
> > > > > > >>
> > > > > > >> On Wed, Apr 6, 2016 at 5:20 PM, Grant Henke <
> > ghe...@cloudera.com>
> > > > > wrote:
> > > > > > >>
> > > > > > >> > KIP-50 as defined is very small. I don't see any harm in
> > putting
> > > > it
> > > > > in
> > > > > > >> as
> > > > > > >> > is and then tackling the follow up work.
> > > > > > >> >
> > > > > > >> >
> > > > > > >> > On Wed, Apr 6, 2016 at 11:16 AM, Ismael Juma <
> > ism...@juma.me.uk>
> > > > > wrote:
> > > > > > >> >
> > > > > > >> > > Thanks Grant. I wonder if KIP-50 should just be done as
> > part of
> > > > > this
> > > > > > >> > work.
> > > > > > >> > >
> > > > > > >> > > Ismael
> > > > > > >> > >
> > > > > > >> > > On Wed, Apr 6, 2016 at 5:12 PM, Grant Henke <
> > > > ghe...@cloudera.com>
> > > > > > >> wrote:
> > > > > > >> > >
> > > > > > >> > > > My work with KIP-4 found that many of the Scala classes
> > used
> > > > in
> > > > > the
> > > > > > >> > > > Authorizer interface are needed in the Clients package
> > when
> > > > > adding
> > > > > > >> the
> > > > > > >> > > > various ACL requests and responses. I also found that we
> > don't
> > > > > have
> > > > > > >> > > > standard Exceptions defined for the authorizer interface.
> > This
> > > > > means
> > > > > > >> > that
> > > > > > >> > > > when I add the Authorizer calls to the broker and wire
> > > > > protocols all
> > > > > > >> > > > exceptions will be reported as an "Unknown Error" back to
> > the
> > > > > user
> > > > > > >> via
> > > > > > >> > > the
> > > > > > >> > > > wire protocol.
> > > > > > >> > > >
> > > > > > >> > > > I have written more about it on the KIP-4 wiki and created
> > > > > jiras to
> > > > > > >> > track
> > > > > > >> > > > those issues (See below). I think we should wrap up this
> > KIP
> > > > as
> > > > > is
> > > > > > >> and
> > > > > > >> > > > tackle the Java/Exception changes as a part of those
> > > > jiras/kips.
> > > > > > >> > > >
> > > > > > >> > > > - KIP-4 "Follow Up Changes"
> > > > > > >> > > > <
> > > > > > >> > > >
> > > > > > >> > >
> > > > > > >> >
> > > > > > >>
> > > > >
> > > >
> > https://cwiki.apache.org/confluence/display/KAFKA/KIP-4+-+Command+line+and+centralized+administrative+operations#KIP-4-Commandlineandcentralizedadministrativeoperations-FollowUpChangesfollow-up-changes
> > > > > > >> > > > >
> > > > > > >> > > > - KAFKA-3509 <
> > > > > https://issues.apache.org/jira/browse/KAFKA-3509>:
> > > > > > >> > > > Provide
> > > > > > >> > > > an Authorizer interface using the Java client
> > enumerator
> > > > > classes
> > > > > > >> > > > - KAFKA-3507 <
> > > > > https://issues.apache.org/jira/browse/KAFKA-3507>:
> > > > > > >> > > Define
> > > > > > >> > > > standard exceptions for the Authorizer interface
> > > > > > >> > > >
> > > > > > >> > > > Thank you,
> > > > > > >> > > > Grant
> > > > > > >> > > >
> > > > > > >> > > > On Wed, Apr 6, 2016 at 10:58 AM, Jay Kreps <
> > j...@confluent.io>
> > > > > > >> wrote:
> > > > > > >> > > >
> > > > > > >> > > > > Hey Ismael,
> > > > > > >> > > > >
> > > > > > >> > > > > Yeah I think this is a minor cleanliness thing. Since
> > this
> > > > is
> > > > > kind
> > > > > > >> > of a
> > > > > > >> > > > > power user interface I don't feel strongly either way.
> > > > > > >> > > > >
> > > > > > >> > > > > My motivation with Scala is just that we've tried to
> > move to
> > > > > > >> having
> > > > > > >> > the
> > > > > > >> > > > > public interfaces be Java, and as a group we definitely
> > > > > struggled
> > > > > > >> a
> > > > > > >> > lot
> > > > > > >> > > > > with understanding and maintaining Scala compatibility
> > in
> > > > the
> > > > > > >> older
> > > > > > >> > > > > clients.
> > > > > > >> > > > >
> > > > > > >> > > > > -Jay
> > > > > > >> > > > >
> > > > > > >> > > > > On Tue, Apr 5, 2016 at 11:46 PM, Ismael Juma <
> > > > > ism...@juma.me.uk>
> > > > > > >> > > wrote:
> > > > > > >> > > > >
> > > > > > >> > > > > > Hi Jay,
> > > > > > >> > > > > >
> > > > > > >> > > > > > On Wed, Apr 6, 2016 at 3:48 AM, Jay Kreps <
> > > > j...@confluent.io
> > > > > >
> > > > > > >> > wrote:
> > > > > > >> > > > > >
> > > > > > >> > > > > > > Given that we're breaking compatibility anyway
> > should
> > > > we:
> > > > > > >> > > > > > >
> > > > > > >> > > > > >
> > > > > > >> > > > > > We are not breaking source compatibility since the new
> > > > > method
> > > > > > >> has a
> > > > > > >> > > > > default
> > > > > > >> > > > > > implementation. I take it that you mean binary
> > > > > compatibility?
> > > > > > >> > > > > >
> > > > > > >> > > > > >
> > > > > > >> > > > > > > 1. Remove the get prefix on this method and the
> > existing
> > > > > one
> > > > > > >> > which
> > > > > > >> > > > > > violate
> > > > > > >> > > > > > > our own code style guidelines (Oops! Kind of sad we
> > went
> > > > > > >> through
> > > > > > >> > > the
> > > > > > >> > > > > > whole
> > > > > > >> > > > > > > KIP process and no one even flagged this)
> > > > > > >> > > > > > >
> > > > > > >> > > > > >
> > > > > > >> > > > > > I did flag this during the discussion and Ashish said
> > he
> > > > > would
> > > > > > >> > change
> > > > > > >> > > > it
> > > > > > >> > > > > if
> > > > > > >> > > > > > other people felt that it should be changed.
> > > > > > >> > > > > >
> > > > > > >> > > > > >
> > > > > > >> > > > > > > 2. Move the interface out of scala to be a normal
> > Java
> > > > > > >> interface
> > > > > > >> > > > > > >
> > > > > > >> > > > > > > This breaks source compatibility but probably what
> > we
> > > > > should
> > > > > > >> have
> > > > > > >> > > > done
> > > > > > >> > > > > > > originally I suspect. Probably there are few enough
> > > > > > >> > implementations
> > > > > > >> > > > of
> > > > > > >> > > > > > this
> > > > > > >> > > > > > > that it is better to just rip the bandaid off.
> > > > > > >> > > > > > >
> > > > > > >> > > > > >
> > > > > > >> > > > > > Can you please explain the motivation? It did come up
> > in
> > > > > > >> previous
> > > > > > >> > > > > > discussions that some things like Operation and
> > > > ResourceType
> > > > > > >> should
> > > > > > >> > > be
> > > > > > >> > > > in
> > > > > > >> > > > > > the clients library, but not Authorizer itself. Are we
> > > > > saying
> > > > > > >> that
> > > > > > >> > > any
> > > > > > >> > > > > > pluggable interface should be in Java so that users
> > can
> > > > > > >> implement
> > > > > > >> > it
> > > > > > >> > > > > > without including the Scala library?
> > > > > > >> > > > > >
> > > > > > >> > > > > > Grant, you originally suggested that some things would
> > > > have
> > > > > to
> > > > > > >> be
> > > > > > >> > in
> > > > > > >> > > > the
> > > > > > >> > > > > > Java side as well. Can you please elaborate on this?
> > > > > > >> > > > > >
> > > > > > >> > > > > > Ismael
> > > > > > >> > > > > >
> > > > > > >> > > > >
> > > > > > >> > > >
> > > > > > >> > > >
> > > > > > >> > > >
> > > > > > >> > > > --
> > > > > > >> > > > Grant Henke
> > > > > > >> > > > Software Engineer | Cloudera
> > > > > > >> > > > gr...@cloudera.com | twitter.com/gchenke |
> > > > > > >> linkedin.com/in/granthenke
> > > > > > >> > > >
> > > > > > >> > >
> > > > > > >> >
> > > > > > >> >
> > > > > > >> >
> > > > > > >> > --
> > > > > > >> > Grant Henke
> > > > > > >> > Software Engineer | Cloudera
> > > > > > >> > gr...@cloudera.com | twitter.com/gchenke |
> > > > > linkedin.com/in/granthenke
> > > > > > >> >
> > > > > > >>
> > > > > > >
> > > > > > >
> > > > > > >
> > > > > > > --
> > > > > > >
> > > > > > > Regards,
> > > > > > > Ashish
> > > > > > >
> > > > > >
> > > > > >
> > > > > >
> > > > > > --
> > > > > >
> > > > > > Regards,
> > > > > > Ashish
> > > > >
> > > >
> > > > --
> > > >
> > > > Regards,
> > > > Ashish
> > > >
> >
>
>
>
> --
>
> Regards,
> Ashish
