omalley commented on code in PR #5145:
URL: https://github.com/apache/hadoop/pull/5145#discussion_r1028638870
##########
hadoop-hdfs-project/hadoop-hdfs-rbf/src/main/java/org/apache/hadoop/hdfs/server/federation/store/driver/impl/StateStoreFileBaseImpl.java:
##########
@@ -88,6 +88,15 @@ protected abstract <T extends BaseRecord> BufferedReader
getReader(
protected abstract <T extends BaseRecord> BufferedWriter getWriter(
String path);
+ /**
Review Comment:
I think that separating this out from getWriter is confusing and
unnecessary. Just make getWriter public in all 3 classes.
##########
hadoop-hdfs-project/hadoop-hdfs-rbf/src/main/java/org/apache/hadoop/hdfs/server/federation/store/driver/impl/StateStoreFileBaseImpl.java:
##########
@@ -88,6 +88,15 @@ protected abstract <T extends BaseRecord> BufferedReader
getReader(
protected abstract <T extends BaseRecord> BufferedWriter getWriter(
String path);
+ /**
Review Comment:
Although do add the VisibleForTesting annotation.
##########
hadoop-hdfs-project/hadoop-hdfs-rbf/src/main/java/org/apache/hadoop/hdfs/server/federation/store/driver/impl/StateStoreFileBaseImpl.java:
##########
@@ -348,25 +357,28 @@ public <T extends BaseRecord> boolean putAll(
for (Entry<String, T> entry : toWrite.entrySet()) {
String recordPath = entry.getKey();
String recordPathTemp = recordPath + "." + now() + TMP_MARK;
- BufferedWriter writer = getWriter(recordPathTemp);
+ BufferedWriter writer = getBufferedWriter(recordPathTemp);
+ boolean recordWrittenSuccessfully = true;
Review Comment:
Initialize the value to false and then only set it to true when the write
completes.
--
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.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
