melissayou commented on code in PR #4311:
URL: https://github.com/apache/hadoop/pull/4311#discussion_r892646952


##########
hadoop-hdfs-project/hadoop-hdfs-client/src/main/java/org/apache/hadoop/hdfs/NameNodeProxiesClient.java:
##########
@@ -349,6 +349,18 @@ public static ClientProtocol 
createProxyWithAlignmentContext(
       boolean withRetries, AtomicBoolean fallbackToSimpleAuth,
       AlignmentContext alignmentContext)
       throws IOException {
+    if (!conf.getBoolean(HdfsClientConfigKeys.DFS_OBSERVER_READ_ENABLE,
+        HdfsClientConfigKeys.DFS_OBSERVER_READ_ENABLE_DEFAULT)) {
+      //Disabled observer read
+      if (alignmentContext == null) {
+        alignmentContext = new ClientGSIContext();
+      }
+      if (alignmentContext instanceof ClientGSIContext) {

Review Comment:
   I might lack context, so we will disable observer read only when 
alignmentContext is null and the observer read is set to false. Can you 
probably explain why we we care to check alignmentContext not null first? If 
it's originally not null, what does that mean? Thank you. 



##########
hadoop-hdfs-project/hadoop-hdfs-client/src/main/java/org/apache/hadoop/hdfs/ClientGSIContext.java:
##########
@@ -40,6 +40,14 @@ public class ClientGSIContext implements AlignmentContext {
   private final LongAccumulator lastSeenStateId =
       new LongAccumulator(Math::max, Long.MIN_VALUE);
 
+  public void disableObserverRead() {
+    if(lastSeenStateId.get() > -1L) {
+      throw new IllegalStateException(
+          "Can't disable observer read after communicate.");
+    }
+    lastSeenStateId.accumulate(-1L);

Review Comment:
   nitpick: Is -1L reserved only for disabled observer read situation or will 
it be used for other (future) cases? If we expect future cases use -2L, -3L, 
etc, will it be helpful to add a comment/doc to explicitly state -1L is 
reserved for disabled observer read situation? 



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

Reply via email to