Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add util to map shard IDs #4175

Merged
merged 3 commits into from Apr 18, 2023
Merged

Conversation

wxing1292
Copy link
Contributor

What changed?

  • Add util to map shard IDs & UT

Why?
More utils

How did you test it?
UT

Potential risks
N/A

Is hotfix candidate?
N/A

@wxing1292 wxing1292 requested a review from a team as a code owner April 17, 2023 21:58
@wxing1292 wxing1292 force-pushed the shard-id-mapping branch 4 times, most recently from 6611836 to e13b0fb Compare April 18, 2023 00:35
@@ -386,6 +386,40 @@ func WorkflowIDToHistoryShard(
return int32(hash%uint32(numberOfShards)) + 1 // ShardID starts with 1
}

func MapShardID(
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you merge with the existing method?

func generateShardIDs(localShardId int32, localShardCount int32, remoteShardCount int32) []int32 {
	var pollingShards []int32
	if remoteShardCount <= localShardCount {
		if localShardId <= remoteShardCount {
			pollingShards = append(pollingShards, localShardId)
		}
		return pollingShards
	}
	// remoteShardCount > localShardCount, replication poller will poll from multiple remote shard.
	for i := localShardId; i <= remoteShardCount; i += localShardCount {
		pollingShards = append(pollingShards, i)
	}
	return pollingShards
}

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

the behavior is actually different, generateShardIDs will not create a replication channel in this case
e.g.

64K shards pulll 16K shards, caller being 16K+1

@wxing1292 wxing1292 enabled auto-merge (squash) April 18, 2023 00:54
@wxing1292 wxing1292 merged commit e792a68 into temporalio:master Apr 18, 2023
9 checks passed
@wxing1292 wxing1292 deleted the shard-id-mapping branch April 18, 2023 01:18
samanbarghi pushed a commit to samanbarghi/temporal that referenced this pull request May 2, 2023
* Add util to map shard IDs & UT
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants