[GitHub] [incubator-celeborn] cfmcgrady commented on a diff in pull request #1613: [CELEBORN-703][WORKER][PERF] Avoid calling `CelebornConf#get` multi-time when `PushDataHandler` handle `PushData`/`PushMergedData`

["cfmcgrady (via GitHub)" <gi...@apache.org>]

cfmcgrady commented on code in PR #1613:
URL: https://github.com/apache/incubator-celeborn/pull/1613#discussion_r1236258196


##########
worker/src/main/scala/org/apache/celeborn/service/deploy/worker/PushDataHandler.scala:
##########
@@ -42,24 +42,27 @@ import org.apache.celeborn.service.deploy.worker.storage.{FileWriter, HdfsFlushe
 
 class PushDataHandler extends BaseMessageHandler with Logging {
 
-  var workerSource: WorkerSource = _
-  var partitionLocationInfo: WorkerPartitionLocationInfo = _
-  var shuffleMapperAttempts: ConcurrentHashMap[String, AtomicIntegerArray] = _
-  var shufflePartitionType: ConcurrentHashMap[String, PartitionType] = _
-  var shufflePushDataTimeout: ConcurrentHashMap[String, Long] = _
-  var replicateThreadPool: ThreadPoolExecutor = _
-  var unavailablePeers: ConcurrentHashMap[WorkerInfo, Long] = _
-  var pushClientFactory: TransportClientFactory = _
-  var registered: AtomicBoolean = new AtomicBoolean(false)
-  var workerInfo: WorkerInfo = _
-  var diskReserveSize: Long = _
-  var partitionSplitMinimumSize: Long = _
-  var shutdown: AtomicBoolean = _
-  var storageManager: StorageManager = _
-  var conf: CelebornConf = _
-  @volatile var pushMasterDataTimeoutTested = false
-  @volatile var pushSlaveDataTimeoutTested = false
-  var workerPartitionSplitEnabled: Boolean = _
+  private var workerSource: WorkerSource = _
+  private var partitionLocationInfo: WorkerPartitionLocationInfo = _
+  private var shuffleMapperAttempts: ConcurrentHashMap[String, AtomicIntegerArray] = _
+  private var shufflePartitionType: ConcurrentHashMap[String, PartitionType] = _
+  private var shufflePushDataTimeout: ConcurrentHashMap[String, Long] = _
+  private var replicateThreadPool: ThreadPoolExecutor = _
+  private var unavailablePeers: ConcurrentHashMap[WorkerInfo, Long] = _
+  private var pushClientFactory: TransportClientFactory = _
+  private var registered: AtomicBoolean = new AtomicBoolean(false)

Review Comment:
   No, the variable `shutdown` is assigned as a reference to `worker.shutdown` during the class initialization process.



-- 
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: issues-unsubscribe@celeborn.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org