----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/73185/#review222604 -----------------------------------------------------------
Fix it, then Ship it! notification/src/main/java/org/apache/atlas/kafka/AtlasKafkaConsumer.java Lines 118 (patched) <https://reviews.apache.org/r/73185/#comment311714> Consider logging following: LOG.info("Skipping already processed message: topic={}, partition={} offset={}. Last processed offset={}", record.topic(), record.partition(), record.offset(), lastCommittedPartitionOffset.get(topicPartition)); notification/src/test/java/org/apache/atlas/notification/AbstractNotificationConsumerTest.java Lines 230 (patched) <https://reviews.apache.org/r/73185/#comment311713> instead of returning null, perhaps return should be: return this.receive(); webapp/src/main/java/org/apache/atlas/notification/NotificationHookConsumer.java Lines 193 (patched) <https://reviews.apache.org/r/73185/#comment311712> Consider marking lastCommittedPartitionOffset as final. - Madhan Neethiraj On Feb. 17, 2021, 6:19 p.m., Ashutosh Mestry wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/73185/ > ----------------------------------------------------------- > > (Updated Feb. 17, 2021, 6:19 p.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 > > webapp/src/test/java/org/apache/atlas/notification/NotificationHookConsumerKafkaTest.java > 33191a7a6 > > > Diff: https://reviews.apache.org/r/73185/diff/3/ > > > Testing > ------- > > **PC Build** > https://ci-builds.apache.org/job/Atlas/job/PreCommit-ATLAS-Build-Test/392/ > > **Functional tests** > Large message payload. > > > Thanks, > > Ashutosh Mestry > >
