Add chunk offset to file id key to make each chunk have a unique key #825

sfc-gh-psaha · 2024-09-11T22:17:37Z

Also add sdk version to Parquet metadata.

src/main/java/net/snowflake/ingest/streaming/internal/ParquetFlusher.java

… Also add sdk version to Parquet metadata.

sfc-gh-tzhang

Change looks good in SDK, please sync with @sfc-gh-kkloudas to make sure this change won't break replication.

sfc-gh-tzhang · 2024-09-16T18:11:43Z

src/main/java/net/snowflake/ingest/streaming/internal/ParquetFlusher.java

+    final String[] parts = shortName.split("\\.");
+    metadata.put(
+        Constants.PRIMARY_FILE_ID_KEY,
+        String.format("%s%s.%s", parts[0], chunkStartOffset, parts[1]));


Is there any issue if the does not match with the file name? Is there any server side change this needs to wait before merging? cc: @sfc-gh-kkloudas

And do you think we can do %s_%s.%s?

I am trying to match with what Andrey will do on the server side - we don't have to match but would be good to be consistent. Let's have @sfc-gh-azagrebin confirm what he'd do. We can wait for the server side change. And no, it does not need to match the file name at all - it can be a UUID for all change tracking cares about.

And if we change the format, is there a use case that customer needs to join with previous values as part of MERGE?

No, this value is supposed to be unique that does not appear in any other files.

I would also vote for this:

And do you think we can do %s_%s.%s?

let me also first try the server side change

just in case to reduce any potential blast radius, I would append only non-zero offset.

We just discussed it with @sfc-gh-lpapke. We have to add "_" because the name w/o extension ends with a number in our case and we can get into a similar problem that was mentioned for regular partitions:
name_10.bdec + offset 110 = name_10110.bdec
name_101.bdec + offset 10 = name_10110.bdec
those 2 are different bdecs but they will be the same PPNs.

or append to ".bdec" that would be the natural delimiter
maybe this is actually the safest option long term

Good catch. I added an _ separator before the .bdec suffix. PTAL. I have also added a check for 0 chunk offset.

src/main/java/net/snowflake/ingest/streaming/internal/ParquetRowBuffer.java

sfc-gh-azagrebin

Thanks Purujit!

src/main/java/net/snowflake/ingest/streaming/internal/ParquetFlusher.java

src/test/java/net/snowflake/ingest/streaming/internal/RowBufferTest.java

…que key (#825)" This reverts commit a107b50.

#848) Revert "Add chunk offset to file id key to make each chunk have a unique key (#825)" This reverts commit a107b50.

sfc-gh-psaha requested review from sfc-gh-tzhang and a team as code owners September 11, 2024 22:17

sfc-gh-psaha requested a review from sfc-gh-azagrebin September 11, 2024 22:17

sfc-gh-psaha force-pushed the psaha-SNOW-1660058-add-chunk-index branch from 74eaa4e to 4165b5a Compare September 11, 2024 22:21

sfc-gh-psaha commented Sep 11, 2024

View reviewed changes

src/main/java/net/snowflake/ingest/streaming/internal/ParquetFlusher.java Outdated Show resolved Hide resolved

sfc-gh-psaha force-pushed the psaha-SNOW-1660058-add-chunk-index branch from 4165b5a to 4ce7ec7 Compare September 16, 2024 18:00

sfc-gh-psaha changed the title ~~Add chunk index to file id key to make each chunk have a unique key~~ Add chunk offset to file id key to make each chunk have a unique key Sep 16, 2024

Add chunk offset to file id key to make each chunk have a unique key.…

6a46ded

… Also add sdk version to Parquet metadata.

sfc-gh-psaha force-pushed the psaha-SNOW-1660058-add-chunk-index branch from 4ce7ec7 to 6a46ded Compare September 16, 2024 18:01

Fix tests and formatting

3f6a5c0

sfc-gh-tzhang approved these changes Sep 16, 2024

View reviewed changes

sfc-gh-psaha force-pushed the psaha-SNOW-1660058-add-chunk-index branch from e978fa1 to 3f6a5c0 Compare September 16, 2024 18:15

sfc-gh-psaha requested a review from sfc-gh-kkloudas September 16, 2024 21:23

Add underscore as separator and reduce blast radius

a76ef42

sfc-gh-azagrebin approved these changes Sep 18, 2024

View reviewed changes

src/main/java/net/snowflake/ingest/streaming/internal/ParquetFlusher.java Outdated Show resolved Hide resolved

src/test/java/net/snowflake/ingest/streaming/internal/RowBufferTest.java Outdated Show resolved Hide resolved

sfc-gh-psaha added 2 commits September 18, 2024 15:47

Refactor function and add more test case

1cc2134

Formatting

a59631b

sfc-gh-psaha enabled auto-merge (squash) September 18, 2024 16:39

sfc-gh-psaha disabled auto-merge September 18, 2024 18:56

sfc-gh-psaha closed this Sep 18, 2024

sfc-gh-psaha reopened this Sep 18, 2024

sfc-gh-psaha enabled auto-merge (squash) September 18, 2024 18:58

sfc-gh-psaha merged commit a107b50 into master Sep 18, 2024
91 checks passed

sfc-gh-psaha deleted the psaha-SNOW-1660058-add-chunk-index branch September 18, 2024 20:04

sfc-gh-psaha added a commit that referenced this pull request Oct 1, 2024

Revert "Add chunk offset to file id key to make each chunk have a uni…

8ad2ee6

…que key (#825)" This reverts commit a107b50.

sfc-gh-psaha mentioned this pull request Oct 1, 2024

Revert "Add chunk offset to file id key to make each chunk have a uni… #848

Merged

sfc-gh-psaha added a commit that referenced this pull request Oct 1, 2024

Revert "Add chunk offset to file id key to make each chunk have a uni… (

0930648

#848) Revert "Add chunk offset to file id key to make each chunk have a unique key (#825)" This reverts commit a107b50.

sfc-gh-xhuang mentioned this pull request Oct 31, 2024

Re-merge "Add chunk offset to file id key to make each chunk have a unique key" from pull 825 #865

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add chunk offset to file id key to make each chunk have a unique key #825

Add chunk offset to file id key to make each chunk have a unique key #825

sfc-gh-psaha commented Sep 11, 2024 •

edited

Loading

sfc-gh-tzhang left a comment

sfc-gh-tzhang Sep 16, 2024

sfc-gh-tzhang Sep 16, 2024

sfc-gh-psaha Sep 16, 2024

sfc-gh-tzhang Sep 16, 2024

sfc-gh-psaha Sep 16, 2024

sfc-gh-azagrebin Sep 17, 2024

sfc-gh-azagrebin Sep 17, 2024

sfc-gh-azagrebin Sep 17, 2024 •

edited

Loading

sfc-gh-psaha Sep 17, 2024

sfc-gh-azagrebin left a comment

Add chunk offset to file id key to make each chunk have a unique key #825

Add chunk offset to file id key to make each chunk have a unique key #825

Conversation

sfc-gh-psaha commented Sep 11, 2024 • edited Loading

sfc-gh-tzhang left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

sfc-gh-azagrebin Sep 17, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

sfc-gh-azagrebin left a comment

Choose a reason for hiding this comment

sfc-gh-psaha commented Sep 11, 2024 •

edited

Loading

sfc-gh-azagrebin Sep 17, 2024 •

edited

Loading