kafka-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Apurva Mehta <apu...@confluent.io>
Subject Re: [DISCUSS] KIP-98: Exactly Once Delivery and Transactional Messaging
Date Tue, 07 Feb 2017 02:01:55 GMT
Hello,

I have gone ahead and updated the KIP wiki with a summary of the changes to
the RPC protocol. The KIP wiki should now have _all_ the public facing
changes being proposed.

The proposed changes were always in the Google doc, and now we are simply
making good on our promise to copy them over to the wiki since the design
is almost finalized.

Thanks,
Apurva

On Mon, Feb 6, 2017 at 4:02 PM, Jason Gustafson <jason@confluent.io> wrote:

> Hey Tom,
>
> Re; complexity. This is always a tradeoff with new features. The changes
> we've made during the design and review process have greatly simplified the
> implementation for clients, and especially for the consumer, but there is
> nontrivial work needed here to support transactions on the producer. I'm
> not sure how it could be otherwise and we've spent a ton of time thinking
> about this. It's also worth mentioning that there's a relatively low bar to
> support the idempotent producer while keeping the client thin (it requires
> support for one new request type and some sequence bookkeeping).
>
> Ultimately, we have to decide whether the improved semantics are worth the
> cost of the complexity. In my opinion, they are. The benefit of having
> exactly-once processing in streaming applications is huge. And to manage
> the complexity, we've intentionally used patterns that were already
> familiar in the codebase (e.g. our approach to maintaining transaction
> state through a coordinator is copied from how consumer offsets are
> managed). We've moved as much work from the clients to the broker as
> possible, and we have resisted at every turn complicating the client APIs
> even where it may have simplified some internals.
>
> -Jason
>
> On Mon, Feb 6, 2017 at 2:55 PM, Apurva Mehta <apurva@confluent.io> wrote:
>
> > Hi Tom,
> >
> > I updated the KIP with a note our plans for performance testing:
> >
> > https://cwiki.apache.org/confluence/display/KAFKA/KIP-
> > 98+-+Exactly+Once+Delivery+and+Transactional+Messaging#KIP-98-
> > ExactlyOnceDeliveryandTransactionalMessaging-Performance
> >
> > Thanks for pointing that out.
> >
> > Regards,
> > Apurva
> >
> >
> >
> >
> >
> > On Mon, Feb 6, 2017 at 7:40 AM, Tom Crayford <tcrayford@heroku.com>
> wrote:
> >
> > > I think the updated wiki page makes sense with respect to ACLs, there
> > seems
> > > to be little potential for abuse there (other than the noted and known
> > > issues).
> > >
> > > I am going to note that this is a major complexity increase for Kafka,
> > and
> > > that I'm concerned about performance impact (the JVM is quite… pedantic
> > > about method size, for example, and even adding conditionals to larger
> > > methods could impact this). The KIP doesn't note plans for performance
> > > testing.
> > >
> > > I'm also concerned about the impact on non-JVM client libraries -
> > writing a
> > > client for Kafka is already a very complicated undertaking, and this
> adds
> > > to that complexity significantly.
> > >
> > > However, the approach seems ok enough. It does also violate the "Kafka
> > has
> > > dumb brokers and smart clients" (which I realize is in direct
> > contradiction
> > > of the previous statement about client implementation being hard). I'd
> > love
> > > to see some discussion in either the doc or the wiki as to why much of
> > this
> > > transactional work isn't a client-side part of Kafka Streams.
> > >
> > > On Sat, Feb 4, 2017 at 3:38 AM, Jason Gustafson <jason@confluent.io>
> > > wrote:
> > >
> > > > One additional note on the authorization. The WriteTxnMarker API is
> > > > restricted to inter-broker usage, so it requires Cluster
> authorization
> > > > (just like other inter-broker APIs). I've updated the document and
> wiki
> > > to
> > > > reflect this.
> > > >
> > > > Also, I have renamed GroupCoordinatorRequest to
> FindCoordinatorRequest
> > > > since there is no group for transactional producers. Let me know if
> > there
> > > > are any concerns.
> > > >
> > > > -Jason
> > > >
> > > > On Fri, Feb 3, 2017 at 2:35 PM, Jason Gustafson <jason@confluent.io>
> > > > wrote:
> > > >
> > > > > Hi Tom,
> > > > >
> > > > > I said this in the voting thread, but can the authors include a
> > section
> > > > >> about new ACLs if there are going to be ACLs for TransactionalId.
> > It's
> > > > >> mentioned in the google doc, but I think new ACLs should be in
a
> KIP
> > > > >> directly.
> > > > >
> > > > >
> > > > > We've updated the wiki. Can you take a look and let us know if you
> > have
> > > > > additional concerns?
> > > > >
> > > > > Thanks,
> > > > > Jason
> > > > >
> > > > > On Fri, Feb 3, 2017 at 1:52 PM, Rajini Sivaram <
> > > rajinisivaram@gmail.com>
> > > > > wrote:
> > > > >
> > > > >> Hi Jason,
> > > > >>
> > > > >> Thank you for the responses. Agree that authorizing
> > transactional.id
> > > in
> > > > >> the
> > > > >> producer requests will be good enough for version 1. And making
it
> > > > tighter
> > > > >> in future based on delegation tokens sounds good too.
> > > > >>
> > > > >> Regards,
> > > > >>
> > > > >> Rajini
> > > > >>
> > > > >>
> > > > >> On Fri, Feb 3, 2017 at 8:04 PM, Jason Gustafson <
> jason@confluent.io
> > >
> > > > >> wrote:
> > > > >>
> > > > >> > Hey Rajini,
> > > > >> >
> > > > >> > Thanks for the questions. Responses below:
> > > > >> >
> > > > >> >
> > > > >> > > 1. Will the transaction coordinator check topic ACLs
based on
> > the
> > > > >> > > requesting client's credentials? Access to transaction
logs,
> > > topics
> > > > >> being
> > > > >> > > added for transaction etc?
> > > > >> >
> > > > >> >
> > > > >> > Good question. I think it makes sense to check topic Write
> > > permission
> > > > >> when
> > > > >> > adding partitions to the transaction. I'll add this to the
> > document.
> > > > >> > Perhaps authorization to the transaction log itself, however,
> can
> > be
> > > > >> > assumed from having access to the ProducerTransactionalId
> > resource?
> > > > This
> > > > >> > would be similar to how access to __consumer_offsets is
assumed
> if
> > > the
> > > > >> > client has access to the Group resource.
> > > > >> >
> > > > >> > 2. If I create a transactional produce request (by hand,
not
> using
> > > the
> > > > >> > > producer API) with a random PID (random, hence unlikely
to be
> in
> > > > use),
> > > > >> > will
> > > > >> > > the broker append a transactional message to the logs,
> > preventing
> > > > LSO
> > > > >> > from
> > > > >> > > moving forward? What validation will broker do for
PIDs?
> > > > >> >
> > > > >> >
> > > > >> > Yes, that is correct. Validation of the TransactionalId
to PID
> > > binding
> > > > >> is a
> > > > >> > known gap in the current proposal, and is discussed in the
> design
> > > > >> document.
> > > > >> > Now that I'm thinking about it a bit more, I think there
is a
> good
> > > > case
> > > > >> for
> > > > >> > including the TransactionalId in the ProduceRequest (I think
Jun
> > > > >> suggested
> > > > >> > this previously). Verifying it does not ensure that the
included
> > PID
> > > > is
> > > > >> > correct, but it does ensure that the client is authorized
to use
> > > > >> > transactions. If the client wanted to do an "endless transaction
> > > > >> attack,"
> > > > >> > having Write access to the topic and an authorized
> transactionalID
> > > is
> > > > >> all
> > > > >> > they would need anyway even if we could authorize the PID
> itself.
> > > This
> > > > >> > seems like a worthwhile improvement.
> > > > >> >
> > > > >> > For future work, my half-baked idea to authorize the PID
binding
> > is
> > > to
> > > > >> > leverage the delegation work in KIP-48. When the PID is
> generated,
> > > we
> > > > >> can
> > > > >> > give the producer a token which is then used in produce
requests
> > > (say
> > > > an
> > > > >> > hmac covering the TransactionalId and PID).
> > > > >> >
> > > > >> >
> > > > >> > > 3. Will every broker check that a client sending transactional
> > > > produce
> > > > >> > > requests at least has write access to transaction log
topic
> > since
> > > it
> > > > >> is
> > > > >> > not
> > > > >> > > validating transactional.id (for every produce request)?
> > > > >> >
> > > > >> >  4. I understand that brokers cannot authorize the transactional
> > id
> > > > for
> > > > >> > each
> > > > >> > > produce request since requests contain only the PID.
But since
> > > there
> > > > >> is a
> > > > >> > > one-to-one mapping between PID and transactional.id,
and a
> > > > >> connection is
> > > > >> > > never expected to change its transactional.id, perhaps
it is
> > > > >> feasible to
> > > > >> > > add authorization and cache the results in the Session?
> Perhaps
> > > not
> > > > >> for
> > > > >> > > version 1, but feels like it will be good to close
the
> security
> > > gap
> > > > >> here.
> > > > >> > > Obviously it would be simpler if transactional.id was
in the
> > > > produce
> > > > >> > > request if the overhead was acceptable.
> > > > >> >
> > > > >> >
> > > > >> > I think my response above addresses both of these. We should
> > include
> > > > the
> > > > >> > TransactionalId in the ProduceRequest. Of course it need
not be
> > > > >> included in
> > > > >> > the message format, so I'm not too concerned about the
> additional
> > > > >> overhead
> > > > >> > it adds.
> > > > >> >
> > > > >> > Thanks,
> > > > >> > Jason
> > > > >> >
> > > > >> >
> > > > >> > On Fri, Feb 3, 2017 at 6:52 AM, Ismael Juma <ismael@juma.me.uk>
> > > > wrote:
> > > > >> >
> > > > >> > > Comments inline.
> > > > >> > >
> > > > >> > > On Thu, Feb 2, 2017 at 6:28 PM, Jason Gustafson <
> > > jason@confluent.io
> > > > >
> > > > >> > > wrote:
> > > > >> > >
> > > > >> > > > Took me a while to remember why we didn't do this.
The
> > timestamp
> > > > >> that
> > > > >> > is
> > > > >> > > > included at the message set level is the max timestamp
of
> all
> > > > >> messages
> > > > >> > in
> > > > >> > > > the message set as is the case in the current
message format
> > (I
> > > > will
> > > > >> > > update
> > > > >> > > > the document to make this explicit). We could
make the
> message
> > > > >> > timestamps
> > > > >> > > > relative to the max timestamp, but that makes
serialization
> a
> > > bit
> > > > >> > awkward
> > > > >> > > > since the timestamps are not assumed to be increasing
> > > sequentially
> > > > >> or
> > > > >> > > > monotonically. Once the messages in the message
set had been
> > > > >> > determined,
> > > > >> > > we
> > > > >> > > > would need to go back and adjust the relative
timestamps.
> > > > >> > > >
> > > > >> > >
> > > > >> > > Yes, I thought this would be a bit tricky and hence
why I
> > > mentioned
> > > > >> the
> > > > >> > > option of adding a new field at the message set level
for the
> > > first
> > > > >> > > timestamp even though that's not ideal either.
> > > > >> > >
> > > > >> > > Here's one idea. We let the timestamps in the messages
be
> > varints,
> > > > >> but we
> > > > >> > > > make their values be relative to the timestamp
of the
> previous
> > > > >> message,
> > > > >> > > > with the timestamp of the first message being
absolute. For
> > > > >> example, if
> > > > >> > > we
> > > > >> > > > had timestamps 500, 501, 499, then we would write
500 for
> the
> > > > first
> > > > >> > > > message, 1 for the next, and -2 for the final
message. Would
> > > that
> > > > >> work?
> > > > >> > > Let
> > > > >> > > > me think a bit about it and see if there are any
problems.
> > > > >> > > >
> > > > >> > >
> > > > >> > > It's an interesting idea. Comparing to the option of
having
> the
> > > > first
> > > > >> > > timestamp in the message set, It's a little more space
> efficient
> > > as
> > > > we
> > > > >> > > don't have both a full timestamp in the message set
_and_ a
> > varint
> > > > in
> > > > >> the
> > > > >> > > first message (which would always be 0, so we avoid
the extra
> > > byte)
> > > > >> and
> > > > >> > > also the deltas could be a little smaller in the common
case.
> > The
> > > > main
> > > > >> > > downside is that it introduces a semantics inconsistency
> between
> > > the
> > > > >> > first
> > > > >> > > message and the rest. Not ideal, but maybe we can live
with
> > that.
> > > > >> > >
> > > > >> > > Ismael
> > > > >> > >
> > > > >> >
> > > > >>
> > > > >
> > > > >
> > > >
> > >
> >
>

Mime
  • Unnamed multipart/alternative (inline, None, 0 bytes)
View raw message