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 2020/05/20 06:31:11 UTC

[GitHub] [samza] bkonold commented on a change in pull request #1364: AzureBlobSystemProducer: Enable adding of number of records in blob as metadata of the blob

bkonold commented on a change in pull request #1364:
URL: https://github.com/apache/samza/pull/1364#discussion_r427769960



##########
File path: samza-azure/src/main/java/org/apache/samza/system/azureblob/utils/BlobMetadataContext.java
##########
@@ -25,10 +25,12 @@
 public class BlobMetadataContext {
   private final String streamName;
   private final long blobSize;
+  private final long numberOfMessagesInBlob;
 
-  public BlobMetadataContext(String streamName, long blobSize) {
+  public BlobMetadataContext(String streamName, long blobSize, long numberOfMessagesInBlob) {
     this.streamName = streamName;
     this.blobSize = blobSize;
+    this.numberOfMessagesInBlob = numberOfMessagesInBlob;

Review comment:
       What will this be used for or what do we anticipate it will be used for?

##########
File path: samza-azure/src/main/java/org/apache/samza/system/azureblob/avro/AzureBlobOutputStream.java
##########
@@ -85,6 +85,7 @@
 
   private volatile boolean isClosed = false;
   private long totalUploadedBlockSize = 0;
+  private long totalNumberOfRecordsInBlob = 0;

Review comment:
       Would it be better to use AtomicLong here to ensure that all access to the member are atomic?

##########
File path: samza-azure/src/main/java/org/apache/samza/system/azureblob/avro/AzureBlobOutputStream.java
##########
@@ -85,6 +85,7 @@
 
   private volatile boolean isClosed = false;
   private long totalUploadedBlockSize = 0;
+  private long totalNumberOfRecordsInBlob = 0;

Review comment:
       Was there any consideration given to using AtomicLong here instead? If threading is involved, it could more clearly express that any modifications will be atomic without the need to worry about method synchronization.




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