Gwen, I don't care what anyone says I think we are totally stlone cold slobar. :-)
I think the only caution I would have is that in general people ask for many things and yet the systems we all admire tend to keep their surface area really really simple. My observation is that never in the history of working on open source has anyone ever asked for simplicity or agitated for removing features, but people really do value that. So I think it is worth trying to really get down to the core problem that the api solves and avoid adding to it unless there is a really clear case. Here are the things I have understood: a. The performance of batching manually could be better due to locking around a batch. This is possible, but I think it would be good to do a quick measurement between the new and old producer and see if this really plays out or not and the magnitude of the performance improvement we could achieve. There were several other perf arguments aside from locking that seemed unlikely to me, but I think a quick measurement could clear all this up. b. It would be nice to have some batch-level atomicity. I agree, but I think this is the cross-partition transaction work. Batching can't really guarantee this (and didn't before) and I think this is one where getting almost what you need (but not quite working) is worse than nothing cause you can't depend on it. c. The code for looping over responses is annoying. I think this is true, but I think if you want to give back the offset and error per message you kind of end up with something like the futures. You could imagine some api that sends a list of messages and returns a Map of errors or something but it is a little special purpose since if you don't care about the error you don't need any special api and if you care about the offset that won't help...so I think to do this really well we need to maybe write down the N patterns of producer usage and see which ones we can improve with a new api. For what it is worth I think a lot of this is just because people were used to the scala API. However the scala api also caused endless confusion because of the weird mixture of manual and automatic batching (what is the difference? when to use one or the other? how do they interact? etc.). -Jay On Wed, Apr 29, 2015 at 6:08 PM, Gwen Shapira <gshap...@cloudera.com> wrote: > I'm starting to think that the old adage "If two people say you are drunk, > lie down" applies here :) > > Current API seems perfectly clear, useful and logical to everyone who wrote > it... but we are getting multiple users asking for the old batch behavior > back. > One reason to get it back is to make upgrades easier - people won't need to > rethink their existing logic if they get an API with the same behavior in > the new producer. The other reason is what Ewen mentioned earlier - if > everyone re-implements Joel's logic, we can provide something for that. > > How about getting the old batch send behavior back by adding a new API > with: > public void batchSend(List<ProducerRecord<K,V>>) > > With this implementation (mixes the old behavior with Joel's snippet): > * send records one by one > * flush > * iterate on futures and "get" them > * log a detailed message on each error > * throw an exception if any send failed. > > It reproduces the old behavior - which apparently everyone really liked, > and I don't think it is overly weird. It is very limited, but anyone who > needs more control over his sends already have plenty of options. > > Thoughts? > > Gwen > > > > > On Tue, Apr 28, 2015 at 5:29 PM, Jay Kreps <jay.kr...@gmail.com> wrote: > > > Hey guys, > > > > The locking argument is correct for very small records (< 50 bytes), > > batching will help here because for small records locking becomes the big > > bottleneck. I think these use cases are rare but not unreasonable. > > > > Overall I'd emphasize that the new producer is way faster at virtually > all > > use cases. If there is a use case where that isn't true, let's look at it > > in a data driven way by comparing the old producer to the new producer > and > > looking for any areas where things got worse. > > > > I suspect the "reducing allocations" argument to be not a big thing. We > do > > a number of small per-message allocations and it didn't seem to have much > > impact. I do think there are a couple of big producer memory > optimizations > > we could do by reusing the arrays in the accumulator in the serialization > > of the request but I don't think this is one of them. > > > > I'd be skeptical of any api that was too weird--i.e. introduces a new way > > of partitioning, gives back errors on a per-partition rather than per > > message basis (given that partitioning is transparent this is really hard > > to think about), etc. Bad apis end up causing a ton of churn and just > don't > > end up being a good long term commitment as we change how the underlying > > code works over time (i.e. we hyper optimize for something then have to > > maintain some super weird api as it becomes hyper unoptimized for the > > client over time). > > > > Roshan--Flush works as you would hope, it blocks on the completion of all > > outstanding requests. Calling get on the future for the request gives you > > the associated error code back. Flush doesn't throw any exceptions > because > > waiting for requests to complete doesn't error, the individual requests > > fail or succeed which is always reported with each request. > > > > Ivan--The batches you send in the scala producer today actually aren't > > truely atomic, they just get sent in a single request. > > > > One tricky problem to solve when user's do batching is size limits on > > requests. This can be very hard to manage since predicting the serialized > > size of a bunch of java objects is not always obvious. This was > repeatedly > > a problem before. > > > > -Jay > > > > On Tue, Apr 28, 2015 at 4:51 PM, Ivan Balashov <ibalas...@gmail.com> > > wrote: > > > > > I must agree with @Roshan – it's hard to imagine anything more > intuitive > > > and easy to use for atomic batching as old sync batch api. Also, it's > > fast. > > > Coupled with a separate instance of producer per > > > broker:port:topic:partition it works very well. I would be glad if it > > finds > > > its way into new producer api. > > > > > > On a side-side-side note, could anyone confirm/deny if SimpleConsumer's > > > fetchSize must be set at least as batch bytes (before or after > > > compression), otherwise client risks not getting any messages? > > > > > >
