----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/73185/#review222593 -----------------------------------------------------------
webapp/src/main/java/org/apache/atlas/notification/NotificationHookConsumer.java Line 589 (original), 590 (patched) <https://reviews.apache.org/r/73185/#comment311705> is 'failedCommitOffsetRecorder' any more needed? Please review. If it is not needed, please remove. webapp/src/main/java/org/apache/atlas/notification/NotificationHookConsumer.java Line 926 (original), 927 (patched) <https://reviews.apache.org/r/73185/#comment311706> I recently saw a deployment using multiple partitions on a topic. To handle such configurations, consider replacing 'long lastCommittedOffset' with: Map<TopicPartition, OffsetAndMetadata> lastCommittedOffset; And send this to AtlasKafkaConsumer.receiveWithCheckedCommit() so that it can get topic/partition specifc offset and commit when appropriate. - Madhan Neethiraj On Feb. 15, 2021, 5:17 a.m., Ashutosh Mestry wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/73185/ > ----------------------------------------------------------- > > (Updated Feb. 15, 2021, 5:17 a.m.) > > > Review request for atlas, Madhan Neethiraj, Nixon Rodrigues, and Sarath > Subramanian. > > > Bugs: ATLAS-4155 > https://issues.apache.org/jira/browse/ATLAS-4155 > > > Repository: atlas > > > Description > ------- > > **Approach** > - Modify: _NotificaitonHookConsumer_: Add lastOffsetToCommit. Update this on > every commit called. Call the new method on > _AtlasKafkaConsumer.receiveWithCheckedCommit_ > - Modify: _AtlasKafkaConsumer_: Add new method _receiveWithCheckedCommit_. > > > Diffs > ----- > > notification/src/main/java/org/apache/atlas/kafka/AtlasKafkaConsumer.java > c38a504fa > > notification/src/main/java/org/apache/atlas/notification/NotificationConsumer.java > f3e81ecc2 > > notification/src/test/java/org/apache/atlas/notification/AbstractNotificationConsumerTest.java > 05d0d8137 > > webapp/src/main/java/org/apache/atlas/notification/NotificationHookConsumer.java > 0e58dacff > > > Diff: https://reviews.apache.org/r/73185/diff/1/ > > > Testing > ------- > > **PC Build** > https://ci-builds.apache.org/job/Atlas/job/PreCommit-ATLAS-Build-Test/392/ > > **Functional tests** > Large message payload. > > > Thanks, > > Ashutosh Mestry > >
