----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/62681/#review186630 -----------------------------------------------------------
notification/src/main/java/org/apache/atlas/notification/AbstractNotification.java Lines 163 (patched) <https://reviews.apache.org/r/62681/#comment263346> This call is already used in Atlas (and in Ranger as well) to generate GUIDs. Do you have suggestions/reference for a more efficient way? notification/src/main/java/org/apache/atlas/notification/AbstractNotification.java Lines 168 (patched) <https://reviews.apache.org/r/62681/#comment263345> It is theoretically possible - especially for tiny messages. For such smaller messages, the execution wouldn't take this path due to "MESSAGE_MAX_LENGHT" check in line #161. This is unlikly for JSON strings, as they will have a lot of duplicate tokens that would result in good amount of size reduction. notification/src/main/java/org/apache/atlas/notification/AlasNotificationMessageDeserializer.java Lines 36 (patched) <https://reviews.apache.org/r/62681/#comment263347> Good catch! Will fix the name. notification/src/main/java/org/apache/atlas/notification/AlasNotificationMessageDeserializer.java Lines 74 (patched) <https://reviews.apache.org/r/62681/#comment263348> Will update to use local variable. This API is called only from Kafka consumer runs - so it is called only from a single thread. However, do you see any issues with multi-threaded access? notification/src/main/java/org/apache/atlas/notification/AlasNotificationMessageDeserializer.java Lines 80 (patched) <https://reviews.apache.org/r/62681/#comment263350> Added a test case in HookMessageDeserializerTest. notification/src/main/java/org/apache/atlas/notification/AlasNotificationMessageDeserializer.java Lines 90 (patched) <https://reviews.apache.org/r/62681/#comment263349> Done. notification/src/main/java/org/apache/atlas/notification/AlasNotificationMessageDeserializer.java Lines 138 (patched) <https://reviews.apache.org/r/62681/#comment263357> Such validatin may not be helpful - as the sender might use a different value for MESSAGE_MAX_LENGTH. - Madhan Neethiraj On Sept. 29, 2017, 12:26 a.m., Madhan Neethiraj wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/62681/ > ----------------------------------------------------------- > > (Updated Sept. 29, 2017, 12:26 a.m.) > > > Review request for atlas, Apoorv Naik, Ashutosh Mestry, Abhay Kulkarni, > Sarath Subramanian, and Suma Shivaprasad. > > > Bugs: ATLAS-2075 > https://issues.apache.org/jira/browse/ATLAS-2075 > > > Repository: atlas > > > Description > ------- > > Update notification module to compress and split large messages (> 1mb) that > can't be sent via Kafka. When such a message is sent, the message is first > compressed via GZip. If compressed message is within the size limit, the > message is sent via Kafka. If the compressed size is larger, then the message > is split into multiple small messages and sent via Kafka. > > Notification consumer is updated to merge and uncompress messages before the > message is processed further. > > > Diffs > ----- > > common/src/main/java/org/apache/atlas/AtlasConfiguration.java 9a9bb76a > notification/src/main/java/org/apache/atlas/kafka/AtlasKafkaConsumer.java > d3b4e49e > notification/src/main/java/org/apache/atlas/kafka/KafkaNotification.java > 38889ef8 > > notification/src/main/java/org/apache/atlas/notification/AbstractMessageDeserializer.java > ec99372d > > notification/src/main/java/org/apache/atlas/notification/AbstractNotification.java > cb44fc68 > > notification/src/main/java/org/apache/atlas/notification/AlasNotificationMessageDeserializer.java > PRE-CREATION > > notification/src/main/java/org/apache/atlas/notification/AtlasNotificationBaseMessage.java > PRE-CREATION > > notification/src/main/java/org/apache/atlas/notification/AtlasNotificationStringMessage.java > PRE-CREATION > > notification/src/main/java/org/apache/atlas/notification/MessageVersion.java > 6ef407ac > > notification/src/main/java/org/apache/atlas/notification/NotificationInterface.java > 956c85e0 > > notification/src/main/java/org/apache/atlas/notification/VersionedMessage.java > 1929eb46 > > notification/src/main/java/org/apache/atlas/notification/VersionedMessageDeserializer.java > cc2099e7 > notification/src/test/java/org/apache/atlas/kafka/KafkaConsumerTest.java > 9b712f44 > > notification/src/test/java/org/apache/atlas/kafka/KafkaNotificationMockTest.java > b7474a0e > > notification/src/test/java/org/apache/atlas/notification/AbstractNotificationConsumerTest.java > 3b2a093b > > notification/src/test/java/org/apache/atlas/notification/AbstractNotificationTest.java > 61107a9f > > notification/src/test/java/org/apache/atlas/notification/VersionedMessageTest.java > 587b7ebd > > notification/src/test/java/org/apache/atlas/notification/entity/EntityMessageDeserializerTest.java > be324277 > > notification/src/test/java/org/apache/atlas/notification/hook/HookMessageDeserializerTest.java > 3724fd5f > > > Diff: https://reviews.apache.org/r/62681/diff/2/ > > > Testing > ------- > > - verified that large messages are compressed and split by Atlas hook, before > sending to Kafka > - verified that Atlas server processes compressed, multi-part messages > correctly > > > Thanks, > > Madhan Neethiraj > >
