Shopify/sarama — pull request #132 by eapache (2014-07-09 16:40:14+00:00)
Title: Potential new Producer design [DO NOT MERGE]

@wvanbergen @burke @Sirupsen @graemej and whoever else might be interested.

This is an idea for a different API for the producer that is somewhat more channel/goroutine based. I was thinking about alternative architectures after reviewing the PR which adds the channels returning failed/successful messages. I started playing with things (after also reviewing the original multiproducer PRs) and came up with this.

It ended up being basically a complete rewrite, so reviewing the diff won't be too helpful, just read the new `producer.go` straight. It's very much a work-in-progress, I'm more interested in architectural thoughts right now than code nits. It seems to pass the simple tests, but that's primarily accidental *shrug*.

Misc. notes on things that might cause questions:
- User communication (input of new messages, receipt of errors) is all done with channels now, so the common idiom I expect would be a `select` on those two channels to avoid blocking.
- It doesn't have a "synchronous" mode, instead the separate `SimpleProducer` uses the `AckSuccesses` flag to effectively imitate that.
- In the normal success path, a message ends up passing through six goroutines before being bundled into a message and passed to the `Broker` object to put on the wire. Not sure if this will impact performance noticeably, but it means that if any intermediate step blocks (i.e. a metadata request for a new leader for a topic) then all other messages not directly impacted will continue processing, which is nice (edit: I'd actually need to add buffering to the intermediate channels for that, but the architecture supports it).
- I delayed actually converting the `Key`/`Value` elements to bytes until the very last moment. This seems cleaner, and also potentially simplifies the addition of streaming messages which somebody asked for recently.
- Hopefully the retry path is easier to reason about than the old one, as it is exactly the same as the normal path now (the message gets resubmitted on exactly the same channel as the user submits messages on, it just has an extra flag set). It should still preserve ordering correctly though (see the `leaderDispatcher` logic for this).

Thoughts? This is obviously non-critical as the current producer seems to be working for our purposes.