[GitHub] [kafka] rohitrmd commented on a change in pull request #9744: KAFKA-10062: Add a method to retrieve the current timestamp as known by the Streams app

[GitBox <gi...@apache.org>]

rohitrmd commented on a change in pull request #9744:
URL: https://github.com/apache/kafka/pull/9744#discussion_r570605728



##########
File path: streams/src/main/java/org/apache/kafka/streams/processor/internals/AbstractProcessorContext.java
##########
@@ -45,7 +45,6 @@
     private boolean initialized;
     protected ProcessorRecordContext recordContext;
     protected ProcessorNode<?, ?, ?, ?> currentNode;
-    private long currentSystemTimeMs;

Review comment:
       @mjsax can you please explain again what is expected now as for me this is contrary to initial KIP. What I have understood from KIP, we wanted to return system time from Stream Task. Considering changes in this pr, 
   
   1. We added one time field in InternalMockProcessorContext which we return when currentSystemTime() is called. 
   2. When ProcessorContextImpl's currentSystemTime() method is called, we return time from streamTask's time field. 
   3. We also added time field in GlobalProcessorContextImpl which we return from currentSystemTime().
   4. If we add new cachedSystemTimeMs field in AbstractProcessorContext, when do you want to return this field? Are earlier changes not valid to return time from StreamTask?
    




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