From 13338c633f12df87187a10a2c5461bcea4a626dd Mon Sep 17 00:00:00 2001 From: Michael Snowden Date: Wed, 25 Jan 2023 09:53:44 -0800 Subject: [PATCH] Revert #3755 --- service/history/shard/context_impl.go | 26 +++++++++--------------- service/history/shard/controller_test.go | 4 +--- 2 files changed, 11 insertions(+), 19 deletions(-) diff --git a/service/history/shard/context_impl.go b/service/history/shard/context_impl.go index f6bba8746fb..22fd909f10c 100644 --- a/service/history/shard/context_impl.go +++ b/service/history/shard/context_impl.go @@ -35,7 +35,6 @@ import ( commonpb "go.temporal.io/api/common/v1" "go.temporal.io/api/enums/v1" "go.temporal.io/api/serviceerror" - "go.uber.org/multierr" "golang.org/x/exp/maps" "go.temporal.io/server/api/adminservice/v1" @@ -1446,7 +1445,8 @@ func (s *ContextImpl) handleReadError(err error) error { case *persistence.ShardOwnershipLostError: // Shard is stolen, trigger shutdown of history engine. // Handling of max read level doesn't matter here. - return multierr.Combine(err, s.transition(contextRequestStop{})) + _ = s.transition(contextRequestStop{}) + return err default: return err @@ -1480,7 +1480,8 @@ func (s *ContextImpl) handleWriteErrorAndUpdateMaxReadLevelLocked(err error, new case *persistence.ShardOwnershipLostError: // Shard is stolen, trigger shutdown of history engine. // Handling of max read level doesn't matter here. - return multierr.Combine(err, s.transition(contextRequestStop{})) + _ = s.transition(contextRequestStop{}) + return err default: // We have no idea if the write failed or will eventually make it to persistence. Try to re-acquire @@ -1489,7 +1490,8 @@ func (s *ContextImpl) handleWriteErrorAndUpdateMaxReadLevelLocked(err error, new // reliably check the outcome by performing a read. If we fail, we'll shut down the shard. // Note that reacquiring the shard will cause the max read level to be updated // to the new range (i.e. past newMaxReadLevel). - return multierr.Combine(err, s.transition(contextRequestLost{})) + _ = s.transition(contextRequestLost{}) + return err } } @@ -1512,24 +1514,18 @@ func (s *ContextImpl) createEngine() Engine { // start should only be called by the controller. func (s *ContextImpl) start() { - if err := s.transition(contextRequestAcquire{}); err != nil { - s.contextTaggedLogger.Error("Failed to start shard", tag.Error(err)) - } + _ = s.transition(contextRequestAcquire{}) } func (s *ContextImpl) Unload() { - if err := s.transition(contextRequestStop{}); err != nil { - s.contextTaggedLogger.Error("Failed to unload shard", tag.Error(err)) - } + _ = s.transition(contextRequestStop{}) } // finishStop should only be called by the controller. func (s *ContextImpl) finishStop() { // After this returns, engineFuture.Set may not be called anymore, so if we don't get see // an Engine here, we won't ever have one. - if err := s.transition(contextRequestFinishStop{}); err != nil { - s.contextTaggedLogger.Error("Failed to stop shard", tag.Error(err)) - } + _ = s.transition(contextRequestFinishStop{}) // use a context that we know is cancelled so that this doesn't block engine, _ := s.engineFuture.Get(s.lifecycleCtx) @@ -2034,9 +2030,7 @@ func (s *ContextImpl) acquireShard() { // On any error, initiate shutting down the shard. If we already changed state // because we got a ShardOwnershipLostError, this won't do anything. - if err := s.transition(contextRequestStop{}); err != nil { - s.contextTaggedLogger.Error("Error stopping shard", tag.Error(err)) - } + _ = s.transition(contextRequestStop{}) } } diff --git a/service/history/shard/controller_test.go b/service/history/shard/controller_test.go index d48a066f75b..e140fcbfdec 100644 --- a/service/history/shard/controller_test.go +++ b/service/history/shard/controller_test.go @@ -790,9 +790,7 @@ func (s *controllerSuite) TestShardControllerFuzz() { shardID := int32(rand.Intn(int(s.config.NumberOfShards))) + 1 switch rand.Intn(5) { case 0: - if _, err := s.shardController.GetShardByID(shardID); err != nil { - return err - } + _, _ = s.shardController.GetShardByID(shardID) case 1: if shard, err := s.shardController.GetShardByID(shardID); err == nil { _, _ = shard.GetEngine(ctx)