mirror of
https://github.com/temporalio/temporal.git
synced 2026-08-30 18:41:49 -07:00
Return early from CHASM PollComponent when shard moves off host (#10878)
## What changed? A blocked `ChasmEngine.pollComponent` now returns a `ShardOwnershipLostError` as soon as its shard moves off this host — by adding a `select` case on the shard's lifecycle context — instead of blocking until the request context deadline. ## Why? Follow-up to #10860 (requested in review): `pollComponent` had the same gap as the `GetWorkflowExecutionHistory` long poll — nothing in its `select` was tied to shard lifecycle, so a poll in flight when its shard moved stalled until timeout. This lets the caller redirect to the new owner immediately. ## How did you test it? - [x] built - [ ] run locally and tested manually - [x] covered by existing tests - [x] added new unit test(s) - [ ] added new functional test(s)
This commit is contained in:
@@ -728,6 +728,11 @@ func (e *ChasmEngine) pollComponent(
|
||||
monotonicPredicate func(chasm.Context, chasm.Component) (bool, error),
|
||||
) (retRef []byte, retError error) {
|
||||
|
||||
shardContext, err := e.getShardContext(ctx, requestRef)
|
||||
if err != nil {
|
||||
return nil, err
|
||||
}
|
||||
|
||||
var ch <-chan struct{}
|
||||
var unsubscribe func()
|
||||
defer func() {
|
||||
@@ -772,6 +777,11 @@ func (e *ChasmEngine) pollComponent(
|
||||
if err != nil || ref != nil {
|
||||
return ref, err
|
||||
}
|
||||
case <-shardContext.GetLifecycleContext().Done():
|
||||
return nil, &persistence.ShardOwnershipLostError{
|
||||
ShardID: shardContext.GetShardID(),
|
||||
Msg: "shard closed",
|
||||
}
|
||||
case <-ctx.Done():
|
||||
return nil, ctx.Err()
|
||||
}
|
||||
|
||||
@@ -18,6 +18,7 @@ import (
|
||||
"go.temporal.io/server/chasm"
|
||||
"go.temporal.io/server/common/cluster"
|
||||
"go.temporal.io/server/common/contextutil"
|
||||
"go.temporal.io/server/common/convert"
|
||||
"go.temporal.io/server/common/dynamicconfig"
|
||||
"go.temporal.io/server/common/membership"
|
||||
"go.temporal.io/server/common/metrics"
|
||||
@@ -1537,6 +1538,70 @@ func (s *chasmEngineSuite) testPollComponentWait(useEmptyRunID bool) {
|
||||
s.Equal(activityID, <-newActivityID)
|
||||
}
|
||||
|
||||
// TestPollComponent_ShardClosed verifies that a poll blocked waiting for notifications returns a
|
||||
// ShardOwnershipLost service error as soon as the shard moves off this host, rather than blocking
|
||||
// until the context deadline.
|
||||
func (s *chasmEngineSuite) TestPollComponent_ShardClosed() {
|
||||
tv := testvars.New(s.T())
|
||||
tv = tv.WithRunID(tv.Any().RunID())
|
||||
|
||||
resolvedKey := chasm.ExecutionKey{
|
||||
NamespaceID: string(tests.NamespaceID),
|
||||
BusinessID: tv.WorkflowID(),
|
||||
RunID: tv.RunID(),
|
||||
}
|
||||
pollRef := chasm.NewComponentRef[*testComponent](resolvedKey)
|
||||
|
||||
s.mockExecutionManager.EXPECT().GetWorkflowExecution(gomock.Any(), gomock.Any()).
|
||||
Return(&persistence.GetWorkflowExecutionResponse{
|
||||
State: s.buildPersistenceMutableState(
|
||||
resolvedKey,
|
||||
&persistencespb.ActivityInfo{},
|
||||
enumsspb.WORKFLOW_EXECUTION_STATE_RUNNING,
|
||||
enumspb.WORKFLOW_EXECUTION_STATUS_RUNNING,
|
||||
nil),
|
||||
}, nil).
|
||||
AnyTimes()
|
||||
|
||||
s.mockShard.Resource.HistoryServiceResolver.EXPECT().
|
||||
Lookup(convert.Int32ToString(s.mockShard.GetShardID())).
|
||||
Return(membership.NewHostInfoFromAddress("owner-host:1234"), nil).
|
||||
Times(1)
|
||||
s.mockShard.Resource.HostInfoProvider.EXPECT().
|
||||
HostInfo().
|
||||
Return(membership.NewHostInfoFromAddress("current-host:5678")).
|
||||
Times(1)
|
||||
|
||||
pollErr := make(chan error, 1)
|
||||
go func() {
|
||||
ctx, cancel := context.WithTimeout(context.Background(), 10*time.Second)
|
||||
defer cancel()
|
||||
// Predicate is never satisfied, so the poll subscribes and blocks.
|
||||
_, err := s.engine.PollComponent(
|
||||
ctx,
|
||||
pollRef,
|
||||
func(chasm.Context, chasm.Component) (bool, error) {
|
||||
return false, nil
|
||||
},
|
||||
)
|
||||
pollErr <- err
|
||||
}()
|
||||
|
||||
// Let the poll park in its select, then move the shard off this host.
|
||||
time.Sleep(100 * time.Millisecond) //nolint:forbidigo
|
||||
s.mockShard.UnloadForOwnershipLost()
|
||||
|
||||
select {
|
||||
case err := <-pollErr:
|
||||
var solErr *serviceerrors.ShardOwnershipLost
|
||||
s.ErrorAs(err, &solErr)
|
||||
s.Equal("owner-host:1234", solErr.OwnerHost)
|
||||
s.Equal("current-host:5678", solErr.CurrentHost)
|
||||
case <-time.After(5 * time.Second):
|
||||
s.FailNow("poll did not return after shard close")
|
||||
}
|
||||
}
|
||||
|
||||
// TestPollComponent_StaleState tests that PollComponent returns a user-friendly Unavailable error
|
||||
// when the submitted component reference is ahead of persisted state (e.g. due to namespace
|
||||
// failover).
|
||||
|
||||
Reference in New Issue
Block a user