[GitHub] mynameborat commented on a change in pull request #915: Consolidating offset read and write for store-offsets and side-inputs, maintaining backward compatbility

[GitBox]

mynameborat commented on a change in pull request #915: Consolidating offset 
read and write for store-offsets and side-inputs, maintaining backward 
compatbility
URL: https://github.com/apache/samza/pull/915#discussion_r255822722
 
 

 ##########
 File path: 
samza-core/src/main/java/org/apache/samza/storage/TaskSideInputStorageManager.java
 ##########
 @@ -65,17 +61,12 @@
  */
 public class TaskSideInputStorageManager {
   private static final Logger LOG = 
LoggerFactory.getLogger(TaskSideInputStorageManager.class);
-  private static final String OFFSET_FILE = "SIDE-INPUT-OFFSETS";
   private static final long STORE_DELETE_RETENTION_MS = 
TimeUnit.DAYS.toMillis(1); // same as changelog delete retention
-  private static final ObjectMapper OBJECT_MAPPER = 
SamzaObjectMapper.getObjectMapper();
-  private static final TypeReference<HashMap<SystemStreamPartition, String>> 
OFFSETS_TYPE_REFERENCE =
-      new TypeReference<HashMap<SystemStreamPartition, String>>() { };
-  private static final ObjectWriter OBJECT_WRITER = 
OBJECT_MAPPER.writerWithType(OFFSETS_TYPE_REFERENCE);
 
   private final Clock clock;
   private final Map<String, SideInputsProcessor> storeToProcessor;
   private final Map<String, StorageEngine> stores;
-  private final String storeBaseDir;
+  private final File storeBaseDir;
 
 Review comment:
   I am guessing you made this change to reuse the helpers between CSM and 
TSSM. I used string to get away from the pattern of using file to represent 
storeBaseDir since we don't use the storeBaseDir for anything else except as a 
prefix to create the absolute offset path.
   
   I kept it simple as a string since it also simplifies testing where it 
didn't make sense to create a file handle to a directory to construct storage 
manager.
   
   Thoughts?
   
   

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services