Skip to content

Commit

Permalink
[CELEBORN-1726] Update WorkerInfo when transition worker state
Browse files Browse the repository at this point in the history
### What changes were proposed in this pull request?
Update WorkerInfo when transition worker state

### Why are the changes needed?
When we send a `getWorkerInfo` request to the `Worker` node after this worker has became `Decommission` state , it does not return the correct state.

note: If we send this request to `Master` node instead of `Worker`, the return value is correct. We should update the worker state also for worker node itself.

### Does this PR introduce _any_ user-facing change?
Yes

### How was this patch tested?
Mannually

Closes #2930 from reswqa/worker_info.

Authored-by: Weijie Guo <[email protected]>
Signed-off-by: Shuang <[email protected]>
  • Loading branch information
reswqa authored and RexXiong committed Nov 20, 2024
1 parent 1d0032b commit bb96a5f
Show file tree
Hide file tree
Showing 2 changed files with 19 additions and 2 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -129,6 +129,7 @@ private[celeborn] class WorkerStatusManager(conf: CelebornConf) extends Logging
if (allowStates != null && allowStates.contains(state)) {
logInfo(s"Worker transition status from ${currentWorkerStatus.getState} to $state.")
currentWorkerStatus = new WorkerStatus(state.getNumber, System.currentTimeMillis())
worker.workerInfo.setWorkerStatus(currentWorkerStatus)
} else {
logWarning(
s"Worker transition status from ${currentWorkerStatus.getState} to $state is not allowed.")
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,13 +19,17 @@ package org.apache.celeborn.service.deploy.worker

import java.util.concurrent.atomic.AtomicBoolean

import com.google.common.collect.Sets
import com.google.common.collect.{Maps, Sets}
import org.junit.Assert
import org.mockito.MockitoSugar._
import org.scalatest.funsuite.AnyFunSuite

import org.apache.celeborn.common.CelebornConf
import org.apache.celeborn.common.identity.UserIdentifier
import org.apache.celeborn.common.meta.WorkerInfo
import org.apache.celeborn.common.protocol.{PbWorkerStatus, WorkerEventType}
import org.apache.celeborn.common.quota.ResourceConsumption
import org.apache.celeborn.common.util.JavaUtils
import org.apache.celeborn.service.deploy.worker.storage.StorageManager

class WorkerStatusManagerSuite extends AnyFunSuite {
Expand All @@ -36,15 +40,27 @@ class WorkerStatusManagerSuite extends AnyFunSuite {
worker = mock[Worker]
val storageManager = mock[StorageManager]
val shuffleKeys = Sets.newHashSet("test")
val workerInfo = new WorkerInfo(
"host",
0,
0,
0,
0,
0,
Maps.newHashMap(),
JavaUtils.newConcurrentHashMap[UserIdentifier, ResourceConsumption])
when(storageManager.shuffleKeySet()).thenReturn(shuffleKeys)
when(worker.storageManager).thenReturn(storageManager)
when(worker.shutdown).thenReturn(new AtomicBoolean())

when(worker.workerInfo).thenReturn(workerInfo)
val statusManager = new WorkerStatusManager(conf)
statusManager.init(worker)

statusManager.doTransition(WorkerEventType.DecommissionThenIdle)
Assert.assertEquals(statusManager.getWorkerState(), PbWorkerStatus.State.InDecommissionThenIdle)
Assert.assertEquals(
worker.workerInfo.getWorkerStatus().getStateValue,
PbWorkerStatus.State.InDecommissionThenIdle.getNumber)

// Rerun state Transition
statusManager.doTransition(WorkerEventType.DecommissionThenIdle)
Expand Down

0 comments on commit bb96a5f

Please sign in to comment.