-
Notifications
You must be signed in to change notification settings - Fork 2k
[ENH] Add the schema for Rust Log Service in Spanner. #6096
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
Conversation
Reviewer ChecklistPlease leverage this checklist to ensure your code review is thorough before approving Testing, Bugs, Errors, Logs, Documentation
System Compatibility
Quality
|
|
Introduce Spanner schema for Rust log service Adds the initial Cloud Spanner schema for the Rust log service, covering manifests, fragments, and fragment regions. Updates the migration manifest sums so the new migration set is tracked with running SHA checksums and refreshed CLI regeneration instructions. Key Changes• Create Affected Areas• rust/spanner-migrations/log_migrations This summary was automatically generated by @propel-code-bot |
rust/spanner-migrations/log_migrations/0001-create_manifests_table.spanner.sql
Outdated
Show resolved
Hide resolved
rust/spanner-migrations/log_migrations/0003-create_fragment_regions_table.spanner.sql
Outdated
Show resolved
Hide resolved
a57a27d to
af49672
Compare
4c34c40 to
a809fdb
Compare
rust/spanner-migrations/log_migrations/0002-create_fragments_table.spanner.sql
Outdated
Show resolved
Hide resolved
af49672 to
5ba021d
Compare
a809fdb to
46ebbf2
Compare
5ba021d to
d1d3137
Compare
46ebbf2 to
4c8c8e1
Compare
d1d3137 to
ed229e0
Compare
d1e28ae to
742a707
Compare
rust/spanner-migrations/log_migrations/0001-create_manifests_table.spanner.sql
Show resolved
Hide resolved
ed229e0 to
2f00206
Compare
049c984 to
9f4eecc
Compare
| collected STRING(64) NOT NULL, | ||
| acc_bytes INT64 NOT NULL, | ||
| writer STRING(64) NOT NULL, | ||
| enumeration_offset INT64 NOT NULL |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I thought you were not in favor of keeping an explicit enumeration_offset since it incurs an update to this table for every write?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This design is the one we benchmarked most recently. The alternative is to do a query across all fragments which is worse.
2f00206 to
06ee7ec
Compare
62532d6 to
e4ea26a
Compare
| CREATE TABLE IF NOT EXISTS fragments ( | ||
| log_id STRING(36) NOT NULL, | ||
| ident STRING(36) NOT NULL, | ||
| path STRING(64) NOT NULL, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[Logic] The path column length of 64 characters is insufficient for the paths generated by wal3.
In rust/wal3/src/lib.rs, unprefixed_fragment_path generates paths like:
log/Bucket={:016x}/FragmentSeqNo={:016x}.parquet
Counting the characters:
log/(4)Bucket=(7)- 16 hex digits (16)
/(1)FragmentSeqNo=(14)- 16 hex digits (16)
.parquet(8)
Total = 66 characters. This exceeds the STRING(64) limit and will cause insertion failures for SeqNo-based fragments.
Increase the column size to at least 128 to be safe.
Context for Agents
The `path` column length of 64 characters is insufficient for the paths generated by `wal3`.
In `rust/wal3/src/lib.rs`, `unprefixed_fragment_path` generates paths like:
`log/Bucket={:016x}/FragmentSeqNo={:016x}.parquet`
Counting the characters:
- `log/` (4)
- `Bucket=` (7)
- 16 hex digits (16)
- `/` (1)
- `FragmentSeqNo=` (14)
- 16 hex digits (16)
- `.parquet` (8)
Total = 66 characters. This exceeds the `STRING(64)` limit and will cause insertion failures for SeqNo-based fragments.
Increase the column size to at least 128 to be safe.
File: rust/spanner-migrations/log_migrations/0002-create_fragments_table.spanner.sql
Line: 4There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
UUID-based fragments don't have the added chars
e4ea26a to
ef9b07f
Compare
Description of changes
This PR is the schema for the replicated log.
Test plan
CI
Migration plan
N/A
Observability plan
N/A
Documentation Changes
N/A