feat: Added support to write parquet files to S3 #5914

malhotrashivam · 2024-08-07T00:19:19Z

Added a new OutputStream to write to S3 and moved all parquet writing code from using SeekableByteChannels to OutputStreams.

devinrsmith

Quick first past.

extensions/parquet/base/src/main/java/io/deephaven/parquet/base/ParquetFileWriter.java

...ns/parquet/table/src/main/java/io/deephaven/parquet/table/ParquetMetadataFileWriterImpl.java

devinrsmith · 2024-08-07T14:18:53Z

extensions/s3/src/main/java/io/deephaven/extensions/s3/S3OutputStream.java

+        private CompletableFuture<UploadPartResponse> future;
+
+        OutgoingRequest(final int partSize) {
+            buffer = ByteBuffer.allocate(partSize);


We could think about taking these from a pool.

Since these buffers are longer lived and we allocate a fixed maximum number of buffers to be used in a circular manner, I didn't use a pool here. But I am open to the idea.

I do think it would be a good idea. Maybe better in a follow-on PR.

extensions/s3/src/main/java/io/deephaven/extensions/s3/S3OutputStream.java

extensions/s3/src/main/java/io/deephaven/extensions/s3/S3SeekableChannelProvider.java

extensions/s3/src/main/java/io/deephaven/extensions/s3/S3Instructions.java

extensions/s3/src/main/java/io/deephaven/extensions/s3/S3OutputStream.java

...ns/parquet/table/src/main/java/io/deephaven/parquet/table/ParquetMetadataFileWriterImpl.java

extensions/parquet/base/src/main/java/io/deephaven/parquet/base/ParquetFileWriter.java

malhotrashivam · 2024-08-07T15:58:37Z

extensions/s3/src/main/java/io/deephaven/extensions/s3/S3OutputStream.java

+        private CompletableFuture<UploadPartResponse> future;
+
+        OutgoingRequest(final int partSize) {
+            buffer = ByteBuffer.allocate(partSize);


Since these buffers are longer lived and we allocate a fixed maximum number of buffers to be used in a circular manner, I didn't use a pool here. But I am open to the idea.

extensions/s3/src/main/java/io/deephaven/extensions/s3/S3OutputStream.java

extensions/s3/src/main/java/io/deephaven/extensions/s3/S3SeekableChannelProvider.java

py/server/deephaven/experimental/s3.py

py/server/deephaven/parquet.py

chipkent

Python LGTM

Util/channel/src/main/java/io/deephaven/util/channel/SeekableChannelsProvider.java

extensions/parquet/base/src/main/java/io/deephaven/parquet/base/ParquetFileWriter.java

rcaudy · 2024-08-09T21:36:03Z

extensions/s3/src/main/java/io/deephaven/extensions/s3/S3OutputStream.java

+        private CompletableFuture<UploadPartResponse> future;
+
+        OutgoingRequest(final int partSize) {
+            buffer = ByteBuffer.allocate(partSize);


I do think it would be a good idea. Maybe better in a follow-on PR.

extensions/s3/src/main/java/io/deephaven/extensions/s3/S3Instructions.java

extensions/s3/src/main/java/io/deephaven/extensions/s3/S3OutputStream.java

chipkent

Py LGTM

Util/channel/src/main/java/io/deephaven/util/channel/LocalFSChannelProvider.java

Util/channel/src/main/java/io/deephaven/util/channel/CompletableOutputStream.java

...ckedfile/src/main/java/io/deephaven/extensions/trackedfile/CompletableLocalOutputStream.java

Util/channel/src/main/java/io/deephaven/util/channel/CompletableOutputStream.java

extensions/s3/src/main/java/io/deephaven/extensions/s3/S3CompletableOutputStream.java

Util/channel/src/main/java/io/deephaven/util/channel/CompletableOutputStream.java

devinrsmith

This was a shallower-than-normal review for purpose of expediency. I can dig in further to specific areas if we think there is value.

extensions/s3/src/main/java/io/deephaven/extensions/s3/S3CompletableOutputStream.java

devinrsmith · 2024-08-15T00:26:22Z

...ckedfile/src/main/java/io/deephaven/extensions/trackedfile/LocalCompletableOutputStream.java

Note: did not review.

deephaven-internal · 2024-08-15T16:56:38Z

Labels indicate documentation is required. Issues for documentation have been opened:

Community: deephaven/deephaven-docs-community#294

malhotrashivam added 7 commits July 26, 2024 05:16

Added support for writing to URIs instead of Files

16885c8

Added a basic S3 writer using aws-s3-outputstream project

054a0e6

Added support to write locally and then push the file to S3

7eb1bab

Minor tweaks

3c08c6c

Added async S3 write support

a8d35bb

Merge branch 'main' into nightly/sm-uri-write

e445c00

Abort S3 upload in case of failure

ebad990

malhotrashivam added feature request New feature or request parquet Related to the Parquet integration DocumentationNeeded ReleaseNotesNeeded Release notes are needed s3 labels Aug 7, 2024

malhotrashivam added this to the 0.36.0 milestone Aug 7, 2024

malhotrashivam self-assigned this Aug 7, 2024

malhotrashivam requested review from chipkent, jmao-denver and rcaudy as code owners August 7, 2024 00:19

devinrsmith reviewed Aug 7, 2024

View reviewed changes

Minor tweaks for clarity

c323bb5

malhotrashivam commented Aug 7, 2024

View reviewed changes

malhotrashivam added 2 commits August 7, 2024 11:32

Review comments part 1

555f64d

Spotless

5f4b340

chipkent reviewed Aug 7, 2024

View reviewed changes

py/server/deephaven/experimental/s3.py Outdated Show resolved Hide resolved

py/server/deephaven/parquet.py Outdated Show resolved Hide resolved

malhotrashivam added 2 commits August 7, 2024 15:25

Review comments part 2

bef6618

Rewording comments

9748b1e

chipkent previously approved these changes Aug 7, 2024

View reviewed changes

Added more comments

ec78cef

malhotrashivam dismissed chipkent’s stale review via ec78cef August 9, 2024 15:58

rcaudy reviewed Aug 9, 2024

View reviewed changes

Moved shadow file logic from ParquetTools to a new output stream

b4af4ca

chipkent previously approved these changes Aug 13, 2024

View reviewed changes

Added more tests

4309c4e

malhotrashivam dismissed chipkent’s stale review via 4309c4e August 13, 2024 17:21

rcaudy reviewed Aug 13, 2024

View reviewed changes

malhotrashivam mentioned this pull request Aug 14, 2024

Use buffer pool to allocate buffers when writing to S3 #5935

Open

malhotrashivam added 2 commits August 14, 2024 12:23

Review comments continued

bfd3f4e

Fixed failing test

9655e4a

rcaudy reviewed Aug 14, 2024

View reviewed changes

Review continued

1c1d62a

devinrsmith reviewed Aug 15, 2024

View reviewed changes

Improved comments and removed some checks

d99af9c

rcaudy approved these changes Aug 15, 2024

View reviewed changes

malhotrashivam merged commit bda75ef into deephaven:main Aug 15, 2024
16 checks passed

github-actions bot locked and limited conversation to collaborators Aug 15, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: Added support to write parquet files to S3 #5914

feat: Added support to write parquet files to S3 #5914

malhotrashivam commented Aug 7, 2024 •

edited

Loading

devinrsmith left a comment

devinrsmith Aug 7, 2024

malhotrashivam Aug 7, 2024

rcaudy Aug 9, 2024

malhotrashivam Aug 14, 2024

malhotrashivam Aug 7, 2024

chipkent left a comment

rcaudy Aug 9, 2024

chipkent left a comment

devinrsmith left a comment

devinrsmith Aug 15, 2024

deephaven-internal commented Aug 15, 2024

feat: Added support to write parquet files to S3 #5914

feat: Added support to write parquet files to S3 #5914

Conversation

malhotrashivam commented Aug 7, 2024 • edited Loading

devinrsmith left a comment

Choose a reason for hiding this comment

devinrsmith Aug 7, 2024

Choose a reason for hiding this comment

malhotrashivam Aug 7, 2024

Choose a reason for hiding this comment

rcaudy Aug 9, 2024

Choose a reason for hiding this comment

malhotrashivam Aug 14, 2024

Choose a reason for hiding this comment

malhotrashivam Aug 7, 2024

Choose a reason for hiding this comment

chipkent left a comment

Choose a reason for hiding this comment

rcaudy Aug 9, 2024

Choose a reason for hiding this comment

chipkent left a comment

Choose a reason for hiding this comment

devinrsmith left a comment

Choose a reason for hiding this comment

devinrsmith Aug 15, 2024

Choose a reason for hiding this comment

deephaven-internal commented Aug 15, 2024

malhotrashivam commented Aug 7, 2024 •

edited

Loading