You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@samza.apache.org by GitBox <gi...@apache.org> on 2021/04/09 01:35:17 UTC

[GitHub] [samza] lakshmi-manasa-g opened a new pull request #1488: SAMZA-2646: Minor refactor of StreamAppender to support sending formats other than byte[] to SystemProducer

lakshmi-manasa-g opened a new pull request #1488:
URL: https://github.com/apache/samza/pull/1488


   Issue: StreamAppender is restrictive in explicitly using byte[] for the serialized log event in the OutgoingMessageEnvelope(OME) sent to underlying SystemProducer. This restricts how the classes extending StreamAppender send to SystemProducer as it prevents sending objects (like avro records) to OME.
   
   Change: Introduce a protected interface to hold the encoded log event and a private impl of it for streamappender.
   
   Tests: existing tests pass, no functionality change.
   
   API changes: none
   
   Usage, upgrade instructions: none


-- 
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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [samza] rmatharu merged pull request #1488: SAMZA-2646: Minor refactor of StreamAppender to support sending formats other than byte[] to SystemProducer

Posted by GitBox <gi...@apache.org>.
rmatharu merged pull request #1488:
URL: https://github.com/apache/samza/pull/1488


   


-- 
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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [samza] rmatharu commented on a change in pull request #1488: SAMZA-2646: Minor refactor of StreamAppender to support sending formats other than byte[] to SystemProducer

Posted by GitBox <gi...@apache.org>.
rmatharu commented on a change in pull request #1488:
URL: https://github.com/apache/samza/pull/1488#discussion_r618740524



##########
File path: samza-log4j2/src/main/java/org/apache/samza/logging/log4j2/StreamAppender.java
##########
@@ -498,4 +498,42 @@ protected void setSerde(Log4jSystemConfig log4jSystemConfig, String systemName)
   public Serde<LogEvent> getSerde() {
     return serde;
   }
+
+  /**
+   * A LogQeueEntry is the element inserted into the log queue of the stream appender
+   * that holds the log messages before they are sent to the underlying system producer.
+   * @param <T> type of object held as the entry
+   */
+  protected interface EncodedLogEvent<T> {
+    /**
+     * fetches the size of the log message held within the LogQueueEntry
+     * @return size of the log message
+     */
+    public long getEntryValueSize();
+
+    /**
+     * fetches the actual log message held within the LogQueueEntry
+     * @return the actual log message
+     */
+    public T getValue();
+  }

Review comment:
       Why define an interface when there is only a single impl?




-- 
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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [samza] lakshmi-manasa-g commented on a change in pull request #1488: SAMZA-2646: Minor refactor of StreamAppender to support sending formats other than byte[] to SystemProducer

Posted by GitBox <gi...@apache.org>.
lakshmi-manasa-g commented on a change in pull request #1488:
URL: https://github.com/apache/samza/pull/1488#discussion_r619490226



##########
File path: samza-log4j2/src/main/java/org/apache/samza/logging/log4j2/StreamAppender.java
##########
@@ -498,4 +498,42 @@ protected void setSerde(Log4jSystemConfig log4jSystemConfig, String systemName)
   public Serde<LogEvent> getSerde() {
     return serde;
   }
+
+  /**
+   * A LogQeueEntry is the element inserted into the log queue of the stream appender
+   * that holds the log messages before they are sent to the underlying system producer.
+   * @param <T> type of object held as the entry
+   */
+  protected interface EncodedLogEvent<T> {
+    /**
+     * fetches the size of the log message held within the LogQueueEntry
+     * @return size of the log message
+     */
+    public long getEntryValueSize();
+
+    /**
+     * fetches the actual log message held within the LogQueueEntry
+     * @return the actual log message
+     */
+    public T getValue();
+  }

Review comment:
       classes extending StreamAppender can provide their own impl --ex to support sending avro records instead of byte[]




-- 
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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org