Use synchronous decompression

[OndroNR]

This fixes #298 which was caused by asynchronous nested message set decompression which resulted in out of order message consuming. Kafka returns messages in MessageSet which contains nested compressed MessageSets of 2000 messages (in my testing). Unfortunately decompression was asynchronous without any synchronisation, so messages were processed out of order in batches of max 2000 (depends on maxTickMessages). My fix applies synchronous decompression.

[estliberitas]

@OndroNR To say, replacing async with async-like sync is no good in general. This is more about how kafka-node does decompression, is it queued or not. And I guess, if you have such a problem, then it does something wrong - w/o queueing.

[estliberitas]

More interested how this is implemented in Java version.

[OndroNR]

@estliberitas would queued async decompression, which adds complexity, come with benefit here? Async decompression in kafka-node is missing queueing, resulting in loosing message order guarantee. Decompression is not I/O dependent, so I don't see any benefit in async.

[estliberitas]

@OndroNR The benefit is not-blocking JS thread. I was trying to tell, that this is not the best solution, just trashing this library with simple workaround which should be solved in a more clean way. And sure with more work, time, etc.

Node.js has zlib.gunzip and zlib.gunzipSync not w/o reason. I'd like to know what's the time of event loop tick when 2-5k messages are processed synchronously.

Let's say someone has application with many-many Websocket connections requiring keep-alive packets to be sent "very often". And then we have a bunch of blocked-enough event loop ticks. And then people will start to profile. And they will see how much time is spent on zlib.gunzipSync. This scenario is not that real, yep, but it's very close to real situation.

Update: another one thing to consider: nodejs/node#1479 (comment). When blocked too long, GC can be called to rarely to collect everything.

[OndroNR]

@estliberitas that make sense. My use case does not suffer from this problem. I will keep this PR here for reference.

[aikar]

This PR can be closed. I've identified the bug that causes messages to be out of order, and it's simply a problem with the decoders logic. Asynchronous can work and still come in order.

I'm going to try to PR a fix next week.

[OndroNR]

This bug is still present in library after such time? We are not consuming Kafka in node.js anymore. I just kept this PR open for reference. I will close it when you create PR.

[aikar]

@OndroNR might as well go ahead and close it since this PR isn't even mergeable anymore.

[OndroNR]

@aikar Alright, closing