chungen0126 commented on code in PR #10573:
URL: https://github.com/apache/ozone/pull/10573#discussion_r3490890577


##########
hadoop-hdds/common/src/main/java/org/apache/hadoop/hdds/scm/pipeline/Pipeline.java:
##########
@@ -170,6 +178,32 @@ public DatanodeID getSuggestedLeaderId() {
     return suggestedLeaderId;
   }
 
+  /**
+   * Return the Pipeline supported StorageTier.
+   *
+   * @return Supported StorageTier
+   */
+  public List<StorageTier> getSupportedStorageTier() {
+    lock.readLock().lock();
+    try {
+      return supportedStorageTier;
+    } finally {
+      lock.readLock().unlock();
+    }
+  }
+
+  /**
+   * Set the storageTier supported by the pipeline.
+   */
+  public void setSupportedStorageTier(List<StorageTier> supportedStorageTier) {

Review Comment:
   > Pipeline has not yet been marked as https://github.com/immutable, and 
there are some other variables that are mutable.
   
   Thanks @xichen01 for the update. Regarding lines 98–111, although Pipeline 
is not currently marked as immutable, the Javadoc suggests it is intended to 
be. Introducing a lock inside the Pipeline object to make a mutable field 
thread-safe is an anti-pattern we should avoid here.
   
   Here is why an Immutable approach is fundamentally better than RRWL 
(ReentrantReadWriteLock):
   1. Read Impact: Every readLock.lock() call still requires a CAS 
(Compare-And-Swap) operation to update the lock status. Under heavy concurrent 
reads, this forces multiple threads to constantly sync for the same memory 
address (lock state), introducing unnecessary overhead. An immutable design 
provides a true zero-cost read, which is critical for SCM's read-heavy workload.
   
   2. Write Impact: Modifying the supported storage tier is an extremely rare 
operation in the pipeline lifecycle. Creating a new Pipeline instance via the 
Builder pattern to update this field is a very lightweight operation.
   



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