> On June 18, 2015, 12:50 a.m., Jason Gustafson wrote: > > core/src/main/scala/kafka/coordinator/ConsumerCoordinator.scala, lines > > 229-236 > > <https://reviews.apache.org/r/35231/diff/1/?file=980918#file980918line229> > > > > In manual group management, would we expect consumerId and generationId > > to be null? > > Guozhang Wang wrote: > In that case, the consumerId would be UNKNOWN_CONSUMER_ID = "", and the > generationId would be -1. These two values are only used inside > OffsetManager.storeOffsets for logging. > > Jason Gustafson wrote: > I wonder if it is worthwhile checking that those values are set > accordingly? If the generationId were 5, for example, would we want to just > commit the offsets blindly? Or would we throw an error? > > Guozhang Wang wrote: > As Onur mentioned, when group == null it is also possible that the group > has not been created on the coordinator (when coordinator migrated, for > example), and in this case the consumerId / generationId would not be ""/-1. > > Jason Gustafson wrote: > That makes sense. I was just thinking this might open the door to having > commits from old or invalid generations go through. Unless we store group > metadata in zookeeper though, perhaps there is no way to prevent it.
So I've been meaning to ask something similar. Guozhang: offline we talked about all offset logic validating generation id before attempting to perform the action. To adjust for this proposed check, at one point we talked about making ConsumerCoordinator more strictly follow the wiki and have the generation id bump happen at the end of rebalance instead of at the beginning so that consumers would be able to commit offsets prior to rebalancing. Given that this rb is about merging in the OffsetManager, should those checks be added here or in a later rb? - Onur ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/35231/#review88301 ----------------------------------------------------------- On June 30, 2015, 1:44 a.m., Guozhang Wang wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/35231/ > ----------------------------------------------------------- > > (Updated June 30, 2015, 1:44 a.m.) > > > Review request for kafka. > > > Bugs: KAFKA-1740 > https://issues.apache.org/jira/browse/KAFKA-1740 > > > Repository: kafka > > > Description > ------- > > v2 > > > minor > > > coordinator response test > > > comments > > > Diffs > ----- > > > clients/src/main/java/org/apache/kafka/clients/consumer/internals/Coordinator.java > 6c26667182d7fa8153469a634881a7c34d8a0c91 > clients/src/main/java/org/apache/kafka/common/protocol/Errors.java > 5b898c8f8ad5d0469f469b600c4b2eb13d1fc662 > > clients/src/main/java/org/apache/kafka/common/requests/OffsetCommitResponse.java > 70844d65369f6ff300cbeb513dbb6650050c7eec > > clients/src/main/java/org/apache/kafka/common/requests/OffsetFetchRequest.java > b5e8a0ff0aaf9df382b91f93e7ad51b1ed5f6209 > > clients/src/main/java/org/apache/kafka/common/requests/OffsetFetchResponse.java > 512a0ef7e619d54e74122c38119209f5cf9590e3 > > clients/src/test/java/org/apache/kafka/clients/consumer/internals/CoordinatorTest.java > 613b192ba84b66f79b45f3cd70418c3f503bee9e > core/src/main/scala/kafka/admin/TopicCommand.scala > dacbdd089f96c67b7bbc1c0cd36490d4fe094c1b > core/src/main/scala/kafka/cluster/Partition.scala > 0990938b33ba7f3bccf373325dbbaee5e45ba8bb > core/src/main/scala/kafka/common/OffsetMetadataAndError.scala > 6b4242c7cd1df9b3465db0fec35a25102c76cd60 > core/src/main/scala/kafka/common/Topic.scala > ad759786d1c22f67c47808c0b8f227eb2b1a9aa8 > core/src/main/scala/kafka/coordinator/ConsumerCoordinator.scala > a385adbd7cb6ed693957df571d175ec36b8eaf94 > core/src/main/scala/kafka/coordinator/CoordinatorMetadata.scala > 0cd5605bcca78dd8a80e8586e58f8b2529da97d7 > core/src/main/scala/kafka/server/KafkaApis.scala > ad6f05807c61c971e5e60d24bc0c87e989115961 > core/src/main/scala/kafka/server/KafkaServer.scala > 52dc728bb1ab4b05e94dc528da1006040e2f28c9 > core/src/main/scala/kafka/server/OffsetManager.scala > 5cca85cf727975f6d3acb2223fd186753ad761dc > core/src/main/scala/kafka/server/ReplicaManager.scala > 59c9bc3ac3a8afc07a6f8c88c5871304db588d17 > core/src/test/scala/integration/kafka/api/ConsumerTest.scala > 17b17b9b1520c7cc2e2ba96cdb1f9ff06e47bcad > core/src/test/scala/integration/kafka/api/IntegrationTestHarness.scala > 07b1ff47bfc3cd3f948c9533c8dc977fa36d996f > core/src/test/scala/unit/kafka/admin/TopicCommandTest.scala > c7136f20972614ac47aa57ab13e3c94ef775a4b7 > core/src/test/scala/unit/kafka/consumer/TopicFilterTest.scala > 4f124af5c3e946045a78ad1519c37372a72c8985 > > core/src/test/scala/unit/kafka/coordinator/ConsumerCoordinatorResponseTest.scala > a44fbd653b53649368db2656c3be3e14e3455163 > core/src/test/scala/unit/kafka/coordinator/CoordinatorMetadataTest.scala > 08854c5e6ec249368206298b2ac2623df18f266a > core/src/test/scala/unit/kafka/server/OffsetCommitTest.scala > 528525b719ec916e16f8b3ae3715bec4b5dcc47d > > Diff: https://reviews.apache.org/r/35231/diff/ > > > Testing > ------- > > > Thanks, > > Guozhang Wang > >
