jolshan commented on code in PR #14845: URL: https://github.com/apache/kafka/pull/14845#discussion_r1414610034
########## group-coordinator/src/main/java/org/apache/kafka/coordinator/group/OffsetMetadataManager.java: ########## @@ -180,7 +185,78 @@ public OffsetMetadataManager build() { /** * The offsets keyed by group id, topic name and partition id. */ - private final TimelineHashMap<String, TimelineHashMap<String, TimelineHashMap<Integer, OffsetAndMetadata>>> offsetsByGroup; + private final Offsets offsets; + + /** + * The offsets keyed by producer id, group id, topic name and partition id. This Review Comment: This comment about the keys is a little confusing. Looking at the code we have 4 maps that are nested within each other. 😅 Maybe the comment could mention that each of these is a separate map? (Or we could include a non java doc comment in offsets to make it clearer. ########## group-coordinator/src/main/java/org/apache/kafka/coordinator/group/OffsetMetadataManager.java: ########## @@ -180,7 +185,78 @@ public OffsetMetadataManager build() { /** * The offsets keyed by group id, topic name and partition id. */ - private final TimelineHashMap<String, TimelineHashMap<String, TimelineHashMap<Integer, OffsetAndMetadata>>> offsetsByGroup; + private final Offsets offsets; + + /** + * The offsets keyed by producer id, group id, topic name and partition id. This Review Comment: This comment about the keys is a little confusing. Looking at the code we have 4 maps that are nested within each other. 😅 Maybe the comment could mention that each of these is a separate map? (Or we could include a non java doc comment in offsets to make it clearer. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: jira-unsubscr...@kafka.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org