try fixing a PartitionConsumer's race condition

[imjustfly]

When closing a PartitionConsumer, a race condition will happen if draining the channel messages . This causes user's consuming routine gets partial messages, which can lead to confusion.

This problem is related to bsm/sarama-cluster#255

[georgeteo]

This fix may break existing users who expect close to drain the message channel and close to return. It may be possible that these users do not actively drain the message channel, so https://github.com/Shopify/sarama/blob/master/consumer.go#L454 may block, which prevents https://github.com/Shopify/sarama/blob/master/consumer.go#L482, which causes https://github.com/Shopify/sarama/blob/master/consumer.go#L431 to block.

[imjustfly]

@georgeteo Yes, you are right.

But I still think it's reasonable not draining messages in Close(). I optimized the compatibility.

[varun06]

@imjustfly What are your view on this PR? is it still relevant?

[imjustfly]

@varun06 Yes, I still think it's relevant.

[varun06]

@sam-obeid you mind taking a quick look?

[varun06]

@imjustfly I am worried about breaking part mentioned by @georgeteo

[imjustfly]

@varun06 The 2nd commit in this PR fixed the compatibility breaking issue. You can check if it works.

[varun06]

Looks good to me 👍 @bai please have a look.

[bai]

Thanks!

[harshana-pickme]

Looks good to me 👍 @bai please have a look.

@varun06 and @imjustfly please have a look at my comment here . Looks like there is a problem in the behaviour.

[varun06]

@harshana-pickme will have a look today. Thanks for pointing out.