[Data] Generate sortable, collision-resistant Dataset IDs by yuhuan130 · Pull Request #65075 · ray-project/ray

yuhuan130 · 2026-07-28T10:58:21Z

Description

This PR replaces counter-based Dataset IDs with 22-character, Base62-encoded ULIDs.

Each ULID contains a timestamp and random data, making it sortable by creation time and practically collision-free.

The full ID format remains:

{dataset_name}_{dataset_ulid}_{run_index}

This PR also:

Generates a new Dataset ULID during deserialization.
Keeps the run index for repeated executions and training epochs.
Adds RAY_DATA_USE_LEGACY_DATASET_IDS as a temporary compatibility option.

Tests

Focused Dataset ID tests: 5 passed

Also updates test_dataset_id_train_ingest test in python/ray/data/tests/test_stats.py to verify that the same generated Dataset ID is used across epochs while the run index increases.

Closes #65073

Signed-off-by: Alex Chien <alexchien130@gmail.com>

gemini-code-assist

Code Review

This pull request introduces time-sortable, practically unique Dataset ULIDs encoded in Base62, replacing the legacy counter-based Dataset IDs by default, while providing a configuration option to fall back to the legacy behavior. The review feedback focuses on improving robustness and compatibility, specifically by replacing parenthesized context managers in tests to maintain Python 3.8 compatibility, optimizing the Base62 encoding function to avoid string concatenation in a loop, and using getattr when accessing use_legacy_dataset_ids to prevent potential AttributeErrors during initialization and deserialization.

bveeramani

Overall LGTM

bveeramani · 2026-08-03T19:52:34Z

+import os
+import time
+
+_BASE62_ALPHABET = "0123456789ABCDEFGHIJKLMNOPQRSTUVWXYZabcdefghijklmnopqrstuvwxyz"


Think this is equivalent?

Suggested change

_BASE62_ALPHABET = "0123456789ABCDEFGHIJKLMNOPQRSTUVWXYZabcdefghijklmnopqrstuvwxyz"

import string

_BASE62_ALPHABET = string.digits + string.ascii_uppercase + string.ascii_lowercase

Also, does this need to be ordered in ASCII order for correctness? If so, maybe worth making that explicit in code + comment

Suggested change

_BASE62_ALPHABET = "0123456789ABCDEFGHIJKLMNOPQRSTUVWXYZabcdefghijklmnopqrstuvwxyz"

_BASE62_ALPHABET = sorted(string.ascii_letters + string.digits)

bveeramani · 2026-08-03T19:53:14Z

+_TIMESTAMP_BITS = 48
+_RANDOM_BITS = 80


Nit: It's unclear if _RANDOM_BITS refers to the number of random its, or a specific value of random bits

Suggested change

_TIMESTAMP_BITS = 48

_RANDOM_BITS = 80

_NUM_TIMESTAMP_BITS = 48

_NUM_RANDOM_BITS = 80

bveeramani · 2026-08-03T19:54:56Z

+    if value < 0:
+        raise ValueError("Cannot encode a negative value in Base62")
+


Nit: _encode_62 is only called internally, and this condition should never happen. Think assertion is probably more appropriate here because it's an internal correctness thing.

Also, including the value is useful for debugging if this assertion ever trips.

Suggested change

if value < 0:

raise ValueError("Cannot encode a negative value in Base62")

assert value >= 0, f"Received a negative value to encode {value}"

bveeramani · 2026-08-03T22:22:37Z

+
+    return encoded.rjust(_ULID_LENGTH, "0")


Nit: Optionally, add a sanity check that the

Suggested change

return encoded.rjust(_ULID_LENGTH, "0")

encoded = encoded.rjust(_ULID_LENGTH, "0")

assert len(encoded) == _ULID_LENGTH, encoded

return encoded

bveeramani · 2026-08-03T22:29:09Z

+    if not 0 <= timestamp_ms < 1 << _TIMESTAMP_BITS:
+        raise ValueError("Timestamp must fit in 48 bits")


This can never happen, right? Make assertion?

bveeramani · 2026-08-03T22:29:46Z

+    if not 0 <= timestamp_ms < 1 << _TIMESTAMP_BITS:
+        raise ValueError("Timestamp must fit in 48 bits")
+
+    random_value = int.from_bytes(os.urandom(_RANDOM_BITS // 8), byteorder="big")


Is byteorder there just because it's a required parameter? Maybe add comments so readers don't get confused about whether this is important for correctness

bveeramani · 2026-08-03T23:05:03Z

    assert sorted(extract_values("id", ds.take())) == list(range(2, 12))


+def test_dataset_lineage_serialization_legacy_dataset_id(


Do we actually need this test? Feel like it's this isn't strictly necessary for correctness

bveeramani · 2026-08-03T23:12:06Z

+BASE62_ALPHABET = "0123456789ABCDEFGHIJKLMNOPQRSTUVWXYZabcdefghijklmnopqrstuvwxyz"
+
+
+def test_generate_dataset_ulid():


Rather than a generic test_generate_dataset_ulid, I think we can split this up into more focused tests that test specific things we care about:

def test_generate_dataset_ulid_returns_time_sortable_ids(): # Optionally, could be reasanoble to make this test more robust/cases # as long as it's still easy to read. ulid1 = generate_dataset_ulid(get_time_ns=lambda: 0) ulid2 = generate_dataset_ulid(get_time_ns=lambda: 1) assert ulid1 < ulid2 def test_generate_dataset_ulid_does_not_return_collisions(): # 100,00 represents an extremely large value where collisions # can't ever practically happen. ulids = {} for _ in range(100_000): ulid = generate_dataset_ulid() ulids.add(ulid) assert len(ulids) == 100_000 def test_generate_dataset_ulid_only_uses_base62_chars(): ulid = generate_dataset_ulid() assert all(c for c in ulid is in BASE62_ALPHABET) def test_generate_dataset_ulid_returns_short_id(): # We care about readabiltiy, so this should always be 22 characters. ulid = generate_dataset_ulid() assert len(ulid) == 22

bveeramani · 2026-08-03T23:12:47Z

+
+from ray.data._internal.dataset_id import generate_dataset_ulid
+
+BASE62_ALPHABET = "0123456789ABCDEFGHIJKLMNOPQRSTUVWXYZabcdefghijklmnopqrstuvwxyz"


Ditto abotu using string module here to simplify

Signed-off-by: Alex Chien <alexchien130@gmail.com>

) `test_track_e2e_training` started failing after #65075 due to a dataset's uuid being regenerated on deserialization rather than using the pickled uuid of the controller. As `test_track_e2e_training` would check the roundtrip dataset info and dataset were identical which is no longer true. Signed-off-by: Mark Towers <mark@anyscale.com> Co-authored-by: Mark Towers <mark@anyscale.com>

feat: data-dataset-ulids

e93118f

Signed-off-by: Alex Chien <alexchien130@gmail.com>

yuhuan130 requested a review from a team as a code owner July 28, 2026 10:58

gemini-code-assist Bot reviewed Jul 28, 2026

View reviewed changes

Comment thread python/ray/data/tests/unit/test_dataset_id.py Outdated

Comment thread python/ray/data/_internal/dataset_id.py Outdated

Comment thread python/ray/data/dataset.py

Comment thread python/ray/data/dataset.py

ray-gardener Bot added data Ray Data-related issues community-contribution Contributed by the community labels Jul 28, 2026

bveeramani self-requested a review August 2, 2026 04:13

bveeramani reviewed Aug 3, 2026

View reviewed changes

fix: Address review feedback

dfa8d64

Signed-off-by: Alex Chien <alexchien130@gmail.com>

bveeramani approved these changes Aug 5, 2026

View reviewed changes

bveeramani added the go add ONLY when ready to merge, run all tests label Aug 5, 2026

bveeramani enabled auto-merge (squash) August 5, 2026 06:19

fix: ci failed tests

d3d4dd3

Signed-off-by: Alex Chien <alexchien130@gmail.com>

github-actions Bot disabled auto-merge August 5, 2026 17:06

bveeramani enabled auto-merge (squash) August 5, 2026 17:41

bveeramani merged commit a9c730c into ray-project:master Aug 5, 2026
7 checks passed

pseudo-rnd-thoughts mentioned this pull request Aug 7, 2026

[Train] Fix test_state.py, uuid regenerated on deserialization #65290

Merged

exalate-issue-sync Bot mentioned this pull request Aug 7, 2026

CI test linux://python/ray/train:test_state is consistently_failing anyscale/ray#39

Closed

elliot-barn mentioned this pull request Aug 7, 2026

[cherry-pick][2.58.0][Train] Fix test_state.py, uuid regenerated on deserialization (#65290) #65294

Open

marwan116 mentioned this pull request Aug 9, 2026

[dashboard] Configurable defaults + UI dialogs for py-spy/memray profiling params #64806

Open

-_BASE62_ALPHABET = "0123456789ABCDEFGHIJKLMNOPQRSTUVWXYZabcdefghijklmnopqrstuvwxyz"
+import string
+_BASE62_ALPHABET = string.digits + string.ascii_uppercase + string.ascii_lowercase

	_BASE62_ALPHABET = "0123456789ABCDEFGHIJKLMNOPQRSTUVWXYZabcdefghijklmnopqrstuvwxyz"
	_BASE62_ALPHABET = sorted(string.ascii_letters + string.digits)

		if value < 0:
		raise ValueError("Cannot encode a negative value in Base62")

		if not 0 <= timestamp_ms < 1 << _TIMESTAMP_BITS:
		raise ValueError("Timestamp must fit in 48 bits")

		assert sorted(extract_values("id", ds.take())) == list(range(2, 12))


		def test_dataset_lineage_serialization_legacy_dataset_id(

		BASE62_ALPHABET = "0123456789ABCDEFGHIJKLMNOPQRSTUVWXYZabcdefghijklmnopqrstuvwxyz"


		def test_generate_dataset_ulid():


		from ray.data._internal.dataset_id import generate_dataset_ulid

		BASE62_ALPHABET = "0123456789ABCDEFGHIJKLMNOPQRSTUVWXYZabcdefghijklmnopqrstuvwxyz"

Uh oh!

Conversation

yuhuan130 commented Jul 28, 2026

Description

Tests

Uh oh!

gemini-code-assist Bot left a comment

Choose a reason for hiding this comment

Code Review

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

bveeramani left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants