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

Introduce FixedConsecutivePartition partition table #527

Merged
merged 6 commits into from
Jun 23, 2023
Merged

Introduce FixedConsecutivePartition partition table #527

merged 6 commits into from
Jun 23, 2023

Conversation

tillrohrmann
Copy link
Contributor

This PR is based on #525.

The FixedConsecutivePartition partition table replaces the FixedPartitionTable.
The latter partition table used a simple round robing partition key to partition
assignment whereas the FixedConsecutivePartition partition table properly maps
partition keys to their respective consecutive partition.

This fixes #524.

@@ -89,6 +89,7 @@ where
}
}

#[instrument(level = "trace", skip_all, fields(peer_id = %self.peer_id, partition_id = %self.partition_id))]
Copy link
Contributor

Choose a reason for hiding this comment

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

Perhaps we can remove the debug log stmt a couple of lines below?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Will remove it.

@@ -58,6 +58,15 @@ pub(crate) struct Transaction<TransactionType> {
inner: TransactionType,
}

impl<TransactionType> Transaction<TransactionType> {
fn assert_partition_key(&self, service_id: &ServiceId) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Add #[inline]

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Will do.

@tillrohrmann
Copy link
Contributor Author

Thanks for the review @slinkydeveloper. Merging this PR now.

This commit changes the PartitionKey to be a u32. This reduces
the required bytes for the partition key while still keeping
enough values to scale arbitrarily.

This fixes #279.
The FixedConsecutivePartition partition table replaces the FixedPartitionTable.
The latter partition table used a simple round robing partition key to partition
assignment whereas the FixedConsecutivePartition partition table properly maps
partition keys to their respective consecutive partition.

This fixes #524.
Since all routable messages carry the PartitionKey, we no longer
need to compute the PartitionKey from the key but can directly use
the stored PartitionKey. This removes some computational complexity
from the routing layer.
@tillrohrmann tillrohrmann merged commit fcebd31 into restatedev:main Jun 23, 2023
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.

VerificationTest > killingTheRuntime failed because runtime tried to insert journal entry twice
2 participants