Skip to content

Support for partitioned identifiers + state mutation request id - #4569

Merged
AhmedSoliman merged 3 commits into
mainfrom
pr4569
Apr 13, 2026
Merged

AhmedSoliman merged 3 commits into
mainfrom
pr4569

Conversation

@AhmedSoliman

@AhmedSoliman AhmedSoliman commented Apr 8, 2026

Copy link
Copy Markdown
Member

@github-actions

github-actions Bot commented Apr 8, 2026

Copy link
Copy Markdown

Test Results

 5 files   -   3   5 suites   - 3   3m 37s ⏱️ +6s
42 tests  -  11  42 ✅  -  11  0 💤 ±0  0 ❌ ±0 
68 runs   - 165  68 ✅  - 165  0 💤 ±0  0 ❌ ±0 

Results for commit 47d77bd. ± Comparison against base commit 7d31cc3.

♻️ This comment has been updated with latest results.

@tillrohrmann tillrohrmann left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Thanks for creating support for partitioned resource ids @AhmedSoliman. The changes look really great. +1 for merging :-)

Comment on lines +147 to +148
// read pkey as u64 (big-endian)
let partition_key = u64::from_be_bytes(b[..size_of::<$crate::PartitionKey>()].try_into().unwrap());

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

big-endian to make the partitioned resource id have the same byte sort order as the deserialized id?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

primarily to maintain compatibility with other ulid-based resource types that use u128 in big-endian as well.

let encoded = id.to_string();
assert!(encoded.starts_with("tst_1"));
assert_eq!(encoded.len(), 38);
println!("{encoded}");

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Is this for visual checks?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Correct.

Comment thread crates/types/src/identifiers/partitioned.rs Outdated
ulid_backed_id!(PartitionProcessorRpcRequest);
ulid_backed_id!(Snapshot @with_resource_id);

partitioned::partitioned_resource_id!(

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Could we theoretically replace the manual ResourceId implementation for the InvocationId with that here?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Yes, but we'll need to do one extra level of indirection (a type to wrap the remainder bytes).

…amps

This timestamp type is intended to be used for invocation scheduling and the `RunAt` field of vqueues.
While it's internally represented as 4-byte integer, it'll be serialized as u63 (yes, missing one bit) in
EntryKey. This is done to allow future expansion of the granularity if needed.


The granularity is chosen to be second-level to (a) reflect the realistic expecations of invocation scheduling
and (b) to allow invocations in queues to be reordered dynamically withing a single-second window based on
their sequence ordering.

Some helpers in this type are not used (floor_to_minute, smear_to_minute) but are kept for future use if we need to
use this type for coarse jittering or if we need to expand the window-bucketing further.
@AhmedSoliman
AhmedSoliman merged commit b55d5e8 into main Apr 13, 2026
16 checks passed
@AhmedSoliman
AhmedSoliman deleted the pr4569 branch April 13, 2026 16:50
@github-actions github-actions Bot locked and limited conversation to collaborators Apr 13, 2026
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants