RexXiong commented on code in PR #3610:
URL: https://github.com/apache/celeborn/pull/3610#discussion_r2867017722


##########
worker/src/main/scala/org/apache/celeborn/service/deploy/worker/storage/StoragePolicy.scala:
##########
@@ -94,6 +94,7 @@ class StoragePolicy(conf: CelebornConf, storageManager: 
StorageManager, source:
     }
 
     def tryCreateFileByType(storageInfoType: StorageInfo.Type): TierWriterBase 
= {
+      val overrideType = if (evict) storageInfoType else 
location.getStorageInfo.getType

Review Comment:
   Suppose location.getStorageInfo.getType is set to HDD and createFileOrder is 
[HDD, S3], if the local HDD fails to create a file, storageInfoType will switch 
to S3. See following code segment:
   
   ```
   for (i <- tryCreateFileTypeIndex until maxSize) {
     val storageStr = order.get(i)
     val storageInfoType = StorageInfo.fromStrToType(storageStr)
     val file = tryCreateFileByType(storageInfoType)
     if (file != null) {
       return file
     }
   }
   ```
   
   However, when storageInfoType is set to S3, the method createDiskFile still 
uses location.getStorageInfo.getType as HDD, which seems incorrect. Therefore, 
StorageType seems should always be overrideType.



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

Reply via email to