Skip to content

ref: Use simple modulo #shards to get the shard number#1476

Merged
olksdr merged 1 commit intomasterfrom
fix/remove-hash-from-kafka-sharding
Sep 15, 2022
Merged

ref: Use simple modulo #shards to get the shard number#1476
olksdr merged 1 commit intomasterfrom
fix/remove-hash-from-kafka-sharding

Conversation

@olksdr
Copy link
Copy Markdown
Contributor

@olksdr olksdr commented Sep 15, 2022

As a follow up for #1454 we decided that we do not have to hash the org id to get the shard number and simple modulo #shard will be sufficient.

As part of this small refactoring the unwrap was also removed and Result is returned instead, to propagate the error to the caller if it happens.

#skip-changelog

As a folllowup for #1454 we decided that we do not have to hash the org
id to get the sahrd number and simple modulo #shard will be sufficient.

As part of this small refactoring the `unwrap` was also removed and
Result is returned instead, to propagate the error to the caller if it
happens.
@olksdr olksdr self-assigned this Sep 15, 2022
@olksdr olksdr requested a review from a team September 15, 2022 10:45
.last()
.map(|(_, v)| v)
.expect("At least one shard is defined");
.ok_or(StoreError::InvalidKafkaShardRange)?;
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

this is an unrelated change right? we don't think this is now more likely

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Yes, this change is unrelated. And error should not happen.
But I also wanted to remove any unwrap/expect since I already make changes to this function.

@jan-auer jan-auer changed the title ref: Use simple modulo #shards to get the shard number. ref: Use simple modulo #shards to get the shard number Sep 15, 2022
@olksdr olksdr merged commit 1a64b6b into master Sep 15, 2022
@olksdr olksdr deleted the fix/remove-hash-from-kafka-sharding branch September 15, 2022 12:12
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.

4 participants