STORM-1858: KafkaBolt: sharing a single producer instance across threads

[vesense]

https://issues.apache.org/jira/browse/STORM-1858

[srdo]

Won't this break support for topologies that use multiple Kafka bolts with different producer properties (e.g. different ack settings)?

[HeartSaVioR]

Maybe we could cache them for each component id, but I'm a bit wondering how many spouts we launch in a worker in production.

[srdo]

Making the producer static also means we need to provide a reset function so test code can reset the variable after each test, and a note in the Javadoc for the bolt that people should remember to reset it when testing.

Edit: Nevermind, wasn't thinking. Resetting isn't an issue :)

@vesense Do you know what kind of performance increase this change gives?

[hmcl]

I wonder how much the gain is by doing this change.

[hmcl]

since you are touching this code, can you please rename the method to mkProducer to newProducer or createProducer. I don't think mk is conventional and/or meaningful.

[harshach]

@vesense not sure any benefit doing this static and making one instance per JVM. This actually adds complexity in code without giving any benefit.
For the most part , when users configures parallelism it means they are trying to spawn the bolt across the workers and possibly across the machines. So any particular that we want to introduce this complexity.

[harshach]

@vesense also if we make this static all the internal producer state becomes shared and this could result in unexpected behavior as per user. Since producer doesn't call flush to broker until the batch.size is met , in a shared state this could mean multiple instances of bolt can quickly fill up the batch.size vs previously where each has their own state. Again we can run into some other unknows as well.