[GitHub] [samza] lakshmi-manasa-g commented on pull request #1392: SAMZA-2556:AzureBlob SystemProducer: Fix NPE thrown during flush. NPE caused by unhandled previous exceptions

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

lakshmi-manasa-g commented on pull request #1392:
URL: https://github.com/apache/samza/pull/1392#issuecomment-775556866


   Answers inline
   
   > 1. Are there other places that might need same handling or is it only the flush method that gets invoked after an uncaught exception?
   
   The AzureBlobSystemProducer has only these operations : start, send, flush and stop. If there is an exception caught in send then flush and/or stop could be called. Both of these operations call flush on the underlying AzureBlobAvroWriter. Hence it would be only in writer.flush that this problem of NPE can occur. Besides the writer.write operation invoked from the send of the producer is already chcking for currentBlobWriterComponents being null. Hence this null check in flush is only other place.
   
   
   >2. Would be good if we can rely on explicit signal instead of null being the signal for uncaught exception.
   >By having an explicit signal, you can enforce the invariant on flush not getting invoked only in the case of unhandled exception but not in other scenarios where the field got nulled.
   
   CurrentBlobWriterComponents is null when the writer is created and remains null until the first message is successfully (correct schema, encoding .. ) written. After the first message CurrentBlobWriterComponents is non null. 
   However, if first message write failed then this is null and an explicit signal (if added) will also be false. Thus i feel an explicit signal will be equivalent to this null check. 
   Added a isClosed check along with null check as writer.flush should not be called after writer.close. This does NOT cause scenario for the current NPE.. just securing and adhering to the writer interface expectations.
   
   >3. Can we have a unit test for this scenario?
   
   added.


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