Removed unneeded sqlite index.

[bbuschkaemper]

SQLite implicitly creates an index due to sample_key being unique. The added index is an unnecessary duplicate resulting in a slowdown, especially for very large (i.e. >1B row) datasets.

[Copilot]

Pull request overview

Removes creation of a redundant explicit SQLite index on samples(sample_key) in the WebDataset SQLite index writer, relying on the implicit index created by the UNIQUE constraint to avoid performance regressions on very large datasets.

Changes:

• Stop creating idx_samples_sample_key on close; rely on the implicit unique index from sample_key TEXT ... UNIQUE.
• Add unit tests ensuring no duplicate sample_key index is created and key-based lookups/duplicate detection still work.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.

File	Description
src/megatron/energon/flavors/webdataset/indexing.py	Removes explicit creation of the redundant idx_samples_sample_key index in SqliteIndexWriter.close().
tests/test_webdataset_indexing.py	Adds tests validating the implicit unique index behavior and ensuring the removed explicit index is not created.

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[Copilot]

Pull request overview

Copilot reviewed 2 out of 2 changed files in this pull request and generated no new comments.

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[philipp-fischer]

@lvoegtle What do you think, do we really need a unit test for sqlite indices?

[bbuschkaemper]

@philipp-fischer I could remove the unit test if you want, was prob an overkill

[voegtlel]

@lvoegtle What do you think, do we really need a unit test for sqlite indices?

Yes, I think we can omit the new additional unittest. This is tested quite thoroughly through the e2e tests.

[voegtlel]

I believe this is actually needed

[voegtlel]

Thank you for this fix! Merging when Tests succeed.