Skip to content

Commit

Permalink
Revert changes to use string casting for shard ID (cadence-workflow#3991
Browse files Browse the repository at this point in the history 
)
  • Loading branch information
@yux0
yux0 authored Feb 17, 2021
1 parent c5334af commit 3c03ea2
Show file tree
Hide file tree
Showing 3 changed files with 17 additions and 18 deletions.
3 changes: 1 addition & 2 deletions client/history/client.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,6 @@ package history

import (
"context"
"strconv"
"sync"
"time"

Expand Down Expand Up @@ -1049,7 +1048,7 @@ func (c *clientImpl) getClientForDomainID(domainID string) (Client, error) {
}

func (c *clientImpl) getClientForShardID(shardID int) (Client, error) {
client, err := c.clients.GetClientForKey(strconv.Itoa(shardID))
client, err := c.clients.GetClientForKey(string(rune(shardID)))
if err != nil {
return nil, err
}
Expand Down
4 changes: 2 additions & 2 deletions service/history/shard/controller.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -278,7 +278,7 @@ func (c *controller) getOrCreateHistoryShardItem(shardID int) (*historyShardsIte
if c.isShuttingDown() || atomic.LoadInt32(&c.status) == common.DaemonStatusStopped {
return nil, fmt.Errorf("controller for host '%v' shutting down", c.GetHostInfo().Identity())
}
info, err := c.GetHistoryServiceResolver().Lookup(fmt.Sprint(shardID))
info, err := c.GetHistoryServiceResolver().Lookup(string(rune(shardID)))
if err != nil {
return nil, err
}
Expand Down Expand Up @@ -378,7 +378,7 @@ func (c *controller) acquireShards() {
if c.isShuttingDown() {
return
}
info, err := c.GetHistoryServiceResolver().Lookup(fmt.Sprint(shardID))
info, err := c.GetHistoryServiceResolver().Lookup(string(rune(shardID)))
if err != nil {
c.logger.Error("Error looking up host for shardID", tag.Error(err), tag.OperationFailed, tag.ShardID(shardID))
} else {
Expand Down
28 changes: 14 additions & 14 deletions service/history/shard/controller_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -112,7 +112,7 @@ func (s *controllerSuite) TestAcquireShardSuccess() {
if hostID == 0 {
myShards = append(myShards, shardID)
s.mockHistoryEngine.EXPECT().Start().Return().Times(1)
s.mockServiceResolver.EXPECT().Lookup(fmt.Sprint(shardID)).Return(s.hostInfo, nil).Times(2)
s.mockServiceResolver.EXPECT().Lookup(string(rune(shardID))).Return(s.hostInfo, nil).Times(2)
s.mockEngineFactory.EXPECT().CreateEngine(gomock.Any()).Return(s.mockHistoryEngine).Times(1)
s.mockShardManager.On("GetShard", mock.Anything, &persistence.GetShardRequest{ShardID: shardID}).Return(
&persistence.GetShardResponse{
Expand Down Expand Up @@ -160,7 +160,7 @@ func (s *controllerSuite) TestAcquireShardSuccess() {
}).Return(nil).Once()
} else {
ownerHost := fmt.Sprintf("test-acquire-shard-host-%v", hostID)
s.mockServiceResolver.EXPECT().Lookup(fmt.Sprint(shardID)).Return(membership.NewHostInfo(ownerHost, nil), nil).Times(1)
s.mockServiceResolver.EXPECT().Lookup(string(rune(shardID))).Return(membership.NewHostInfo(ownerHost, nil), nil).Times(1)
}
}

Expand Down Expand Up @@ -195,7 +195,7 @@ func (s *controllerSuite) TestAcquireShardsConcurrently() {
if hostID == 0 {
myShards = append(myShards, shardID)
s.mockHistoryEngine.EXPECT().Start().Return().Times(1)
s.mockServiceResolver.EXPECT().Lookup(fmt.Sprint(shardID)).Return(s.hostInfo, nil).Times(2)
s.mockServiceResolver.EXPECT().Lookup(string(rune(shardID))).Return(s.hostInfo, nil).Times(2)
s.mockEngineFactory.EXPECT().CreateEngine(gomock.Any()).Return(s.mockHistoryEngine).Times(1)
s.mockShardManager.On("GetShard", mock.Anything, &persistence.GetShardRequest{ShardID: shardID}).Return(
&persistence.GetShardResponse{
Expand Down Expand Up @@ -243,7 +243,7 @@ func (s *controllerSuite) TestAcquireShardsConcurrently() {
}).Return(nil).Once()
} else {
ownerHost := fmt.Sprintf("test-acquire-shard-host-%v", hostID)
s.mockServiceResolver.EXPECT().Lookup(fmt.Sprint(shardID)).Return(membership.NewHostInfo(ownerHost, nil), nil).Times(1)
s.mockServiceResolver.EXPECT().Lookup(string(rune(shardID))).Return(membership.NewHostInfo(ownerHost, nil), nil).Times(1)
}
}

Expand All @@ -263,12 +263,12 @@ func (s *controllerSuite) TestAcquireShardLookupFailure() {
numShards := 2
s.config.NumberOfShards = numShards
for shardID := 0; shardID < numShards; shardID++ {
s.mockServiceResolver.EXPECT().Lookup(fmt.Sprint(shardID)).Return(nil, errors.New("ring failure")).Times(1)
s.mockServiceResolver.EXPECT().Lookup(string(rune(shardID))).Return(nil, errors.New("ring failure")).Times(1)
}

s.shardController.acquireShards()
for shardID := 0; shardID < numShards; shardID++ {
s.mockServiceResolver.EXPECT().Lookup(fmt.Sprint(shardID)).Return(nil, errors.New("ring failure")).Times(1)
s.mockServiceResolver.EXPECT().Lookup(string(rune(shardID))).Return(nil, errors.New("ring failure")).Times(1)
s.Nil(s.shardController.GetEngineForShard(shardID))
}
}
Expand All @@ -285,7 +285,7 @@ func (s *controllerSuite) TestAcquireShardRenewSuccess() {

for shardID := 0; shardID < numShards; shardID++ {
s.mockHistoryEngine.EXPECT().Start().Return().Times(1)
s.mockServiceResolver.EXPECT().Lookup(fmt.Sprint(shardID)).Return(s.hostInfo, nil).Times(2)
s.mockServiceResolver.EXPECT().Lookup(string(rune(shardID))).Return(s.hostInfo, nil).Times(2)
s.mockEngineFactory.EXPECT().CreateEngine(gomock.Any()).Return(s.mockHistoryEngine).Times(1)
s.mockShardManager.On("GetShard", mock.Anything, &persistence.GetShardRequest{ShardID: shardID}).Return(
&persistence.GetShardResponse{
Expand Down Expand Up @@ -339,7 +339,7 @@ func (s *controllerSuite) TestAcquireShardRenewSuccess() {
s.shardController.acquireShards()

for shardID := 0; shardID < numShards; shardID++ {
s.mockServiceResolver.EXPECT().Lookup(fmt.Sprint(shardID)).Return(s.hostInfo, nil).Times(1)
s.mockServiceResolver.EXPECT().Lookup(string(rune(shardID))).Return(s.hostInfo, nil).Times(1)
}
s.shardController.acquireShards()

Expand All @@ -360,7 +360,7 @@ func (s *controllerSuite) TestAcquireShardRenewLookupFailed() {

for shardID := 0; shardID < numShards; shardID++ {
s.mockHistoryEngine.EXPECT().Start().Return().Times(1)
s.mockServiceResolver.EXPECT().Lookup(fmt.Sprint(shardID)).Return(s.hostInfo, nil).Times(2)
s.mockServiceResolver.EXPECT().Lookup(string(rune(shardID))).Return(s.hostInfo, nil).Times(2)
s.mockEngineFactory.EXPECT().CreateEngine(gomock.Any()).Return(s.mockHistoryEngine).Times(1)
s.mockShardManager.On("GetShard", mock.Anything, &persistence.GetShardRequest{ShardID: shardID}).Return(
&persistence.GetShardResponse{
Expand Down Expand Up @@ -414,7 +414,7 @@ func (s *controllerSuite) TestAcquireShardRenewLookupFailed() {
s.shardController.acquireShards()

for shardID := 0; shardID < numShards; shardID++ {
s.mockServiceResolver.EXPECT().Lookup(fmt.Sprint(shardID)).Return(nil, errors.New("ring failure")).Times(1)
s.mockServiceResolver.EXPECT().Lookup(string(rune(shardID))).Return(nil, errors.New("ring failure")).Times(1)
}
s.shardController.acquireShards()

Expand Down Expand Up @@ -461,7 +461,7 @@ func (s *controllerSuite) TestHistoryEngineClosed() {
for shardID := 0; shardID < 2; shardID++ {
mockEngine := historyEngines[shardID]
mockEngine.EXPECT().Stop().Return().Times(1)
s.mockServiceResolver.EXPECT().Lookup(fmt.Sprint(shardID)).Return(differentHostInfo, nil).AnyTimes()
s.mockServiceResolver.EXPECT().Lookup(string(rune(shardID))).Return(differentHostInfo, nil).AnyTimes()
s.shardController.shardClosedCallback(shardID, nil)
}

Expand Down Expand Up @@ -506,7 +506,7 @@ func (s *controllerSuite) TestHistoryEngineClosed() {
for shardID := 2; shardID < numShards; shardID++ {
mockEngine := historyEngines[shardID]
mockEngine.EXPECT().Stop().Return().Times(1)
s.mockServiceResolver.EXPECT().Lookup(fmt.Sprint(shardID)).Return(s.hostInfo, nil).AnyTimes()
s.mockServiceResolver.EXPECT().Lookup(string(rune(shardID))).Return(s.hostInfo, nil).AnyTimes()
}
s.shardController.Stop()
}
Expand Down Expand Up @@ -553,7 +553,7 @@ func (s *controllerSuite) TestShardControllerClosed() {
for shardID := 0; shardID < numShards; shardID++ {
mockEngine := historyEngines[shardID]
mockEngine.EXPECT().Stop().Times(1)
s.mockServiceResolver.EXPECT().Lookup(fmt.Sprint(shardID)).Return(s.hostInfo, nil).AnyTimes()
s.mockServiceResolver.EXPECT().Lookup(string(rune(shardID))).Return(s.hostInfo, nil).AnyTimes()
}
s.shardController.Stop()
workerWG.Wait()
Expand All @@ -570,7 +570,7 @@ func (s *controllerSuite) setupMocksForAcquireShard(shardID int, mockEngine *eng

// s.mockResource.ExecutionMgr.On("Close").Return()
mockEngine.EXPECT().Start().Times(1)
s.mockServiceResolver.EXPECT().Lookup(fmt.Sprint(shardID)).Return(s.hostInfo, nil).Times(2)
s.mockServiceResolver.EXPECT().Lookup(string(rune(shardID))).Return(s.hostInfo, nil).Times(2)
s.mockEngineFactory.EXPECT().CreateEngine(gomock.Any()).Return(mockEngine).Times(1)
s.mockShardManager.On("GetShard", mock.Anything, &persistence.GetShardRequest{ShardID: shardID}).Return(
&persistence.GetShardResponse{
Expand Down

0 comments on commit 3c03ea2

Please sign in to comment.