cerner / cerner_kafka

A Kafka Cookbook for Chef
Apache License 2.0
30 stars 25 forks source link

#33: Updated defaults to kafka 0.9 #34

Closed bbaugher closed 8 years ago

mkwhitacre commented 8 years ago

If we know the offset monitor isn't going to work with the default should we add some warning/logging/error out to this effect so consumers know as well?

noslowerdna commented 8 years ago

Maybe also bump default offset monitor version to latest (0.2.1)

noslowerdna commented 8 years ago

In README might link to http://kafka.apache.org/documentation.html#upgrade_9, as it's critical to temporarily set inter.broker.protocol.version=0.8.2.X for a rolling upgrade.

mkwhitacre commented 8 years ago

Changing the default seems like a major version bump.

bbaugher commented 8 years ago

We already did a major version bump when we remove a lot of the default attributes (https://github.com/cerner/cerner_kafka/commit/5674e21e519d65c26b566d108f8f446ec1f115a1).

bbaugher commented 8 years ago

If we know the offset monitor isn't going to work with the default should we add some warning/logging/error out to this effect so consumers know as well?

Are you referring to the switch to offset storage in kafka? It looks like by default its still zookeeper (doc look for offsets.storage).

bbaugher commented 8 years ago

Addressed all comments with this, https://github.com/cerner/cerner_kafka/commit/c0955cc46b0384d0bdf0c3b3f0f17404edf7a51b

bbaugher commented 8 years ago

Added upgrade guide to call out what we changed and was non-passive or what they should look out for, https://github.com/cerner/cerner_kafka/commit/f507c8014874a46acb4f1a15cc2daba9244c0d62

mkwhitacre commented 8 years ago

+1

noslowerdna commented 8 years ago

+1

bbaugher commented 8 years ago

Merging with enough +1s