Add glacier skipping logic #21164

bangtim · 2024-03-19T16:50:35Z

Description

This PR supersedes this one --> #18237

TrinoS3FileSystem contains logic/configuration for skipping Glacier objects found in S3.

This PR adds the missing functionality/configuration from TrinoS3FileSystem and also adds the capability to configure the filesystem to read restored Glacier objects.

Additional context and related issues

Recently, S3 added the RestoreStatus to the result of ListObjectsV2 - see https://aws.amazon.com/about-aws/whats-new/2023/06/amazon-s3-restore-status-s3-glacier-objects-list-api/ for more details.

Release notes

( ) This is not user-visible or is docs only, and no release notes are required.
( ) Release notes are required. Please propose a release note for me.
(x) Release notes are required, with the following suggested text:

# Hive
* Add support for reading S3 objects restored from glacier storage ({issue}`21164`)

cla-bot · 2024-03-19T16:50:39Z

Thank you for your pull request and welcome to the Trino community. We require contributors to sign our Contributor License Agreement, and we don't seem to have you on file. Continue to work with us on the review and improvements in this PR, and submit the signed CLA to cla@trino.io. Photos, scans, or digitally-signed PDF files are all suitable. Processing may take a few days. The CLA needs to be on file before we merge your changes. For more information, see https://github.com/trinodb/cla

lib/trino-filesystem-s3/src/main/java/io/trino/filesystem/s3/S3ObjectStorageClassFilter.java

cla-bot · 2024-03-19T18:46:29Z

Thank you for your pull request and welcome to the Trino community. We require contributors to sign our Contributor License Agreement, and we don't seem to have you on file. Continue to work with us on the review and improvements in this PR, and submit the signed CLA to cla@trino.io. Photos, scans, or digitally-signed PDF files are all suitable. Processing may take a few days. The CLA needs to be on file before we merge your changes. For more information, see https://github.com/trinodb/cla

cla-bot · 2024-03-19T19:25:49Z

Thank you for your pull request and welcome to the Trino community. We require contributors to sign our Contributor License Agreement, and we don't seem to have you on file. Continue to work with us on the review and improvements in this PR, and submit the signed CLA to cla@trino.io. Photos, scans, or digitally-signed PDF files are all suitable. Processing may take a few days. The CLA needs to be on file before we merge your changes. For more information, see https://github.com/trinodb/cla

cla-bot · 2024-03-19T19:27:07Z

Thank you for your pull request and welcome to the Trino community. We require contributors to sign our Contributor License Agreement, and we don't seem to have you on file. Continue to work with us on the review and improvements in this PR, and submit the signed CLA to cla@trino.io. Photos, scans, or digitally-signed PDF files are all suitable. Processing may take a few days. The CLA needs to be on file before we merge your changes. For more information, see https://github.com/trinodb/cla

cla-bot · 2024-03-21T16:34:35Z

Thank you for your pull request and welcome to the Trino community. We require contributors to sign our Contributor License Agreement, and we don't seem to have you on file. Continue to work with us on the review and improvements in this PR, and submit the signed CLA to cla@trino.io. Photos, scans, or digitally-signed PDF files are all suitable. Processing may take a few days. The CLA needs to be on file before we merge your changes. For more information, see https://github.com/trinodb/cla

cla-bot · 2024-03-21T16:45:38Z

Thank you for your pull request and welcome to the Trino community. We require contributors to sign our Contributor License Agreement, and we don't seem to have you on file. Continue to work with us on the review and improvements in this PR, and submit the signed CLA to cla@trino.io. Photos, scans, or digitally-signed PDF files are all suitable. Processing may take a few days. The CLA needs to be on file before we merge your changes. For more information, see https://github.com/trinodb/cla

electrum · 2024-03-22T20:31:47Z

Please see my comment on the previous PR: #18237 (comment)

cla-bot · 2024-03-28T17:23:35Z

Thank you for your pull request and welcome to the Trino community. We require contributors to sign our Contributor License Agreement, and we don't seem to have you on file. Continue to work with us on the review and improvements in this PR, and submit the signed CLA to cla@trino.io. Photos, scans, or digitally-signed PDF files are all suitable. Processing may take a few days. The CLA needs to be on file before we merge your changes. For more information, see https://github.com/trinodb/cla

cla-bot · 2024-03-28T17:38:09Z

Thank you for your pull request and welcome to the Trino community. We require contributors to sign our Contributor License Agreement, and we don't seem to have you on file. Continue to work with us on the review and improvements in this PR, and submit the signed CLA to cla@trino.io. Photos, scans, or digitally-signed PDF files are all suitable. Processing may take a few days. The CLA needs to be on file before we merge your changes. For more information, see https://github.com/trinodb/cla

cla-bot · 2024-03-28T18:15:21Z

Thank you for your pull request and welcome to the Trino community. We require contributors to sign our Contributor License Agreement, and we don't seem to have you on file. Continue to work with us on the review and improvements in this PR, and submit the signed CLA to cla@trino.io. Photos, scans, or digitally-signed PDF files are all suitable. Processing may take a few days. The CLA needs to be on file before we merge your changes. For more information, see https://github.com/trinodb/cla

cla-bot · 2024-03-28T18:16:11Z

Thank you for your pull request and welcome to the Trino community. We require contributors to sign our Contributor License Agreement, and we don't seem to have you on file. Continue to work with us on the review and improvements in this PR, and submit the signed CLA to cla@trino.io. Photos, scans, or digitally-signed PDF files are all suitable. Processing may take a few days. The CLA needs to be on file before we merge your changes. For more information, see https://github.com/trinodb/cla

cla-bot · 2024-03-28T18:21:57Z

Thank you for your pull request and welcome to the Trino community. We require contributors to sign our Contributor License Agreement, and we don't seem to have you on file. Continue to work with us on the review and improvements in this PR, and submit the signed CLA to cla@trino.io. Photos, scans, or digitally-signed PDF files are all suitable. Processing may take a few days. The CLA needs to be on file before we merge your changes. For more information, see https://github.com/trinodb/cla

cla-bot · 2024-03-28T18:25:42Z

Thank you for your pull request and welcome to the Trino community. We require contributors to sign our Contributor License Agreement, and we don't seem to have you on file. Continue to work with us on the review and improvements in this PR, and submit the signed CLA to cla@trino.io. Photos, scans, or digitally-signed PDF files are all suitable. Processing may take a few days. The CLA needs to be on file before we merge your changes. For more information, see https://github.com/trinodb/cla

cla-bot · 2024-03-28T20:36:57Z

Thank you for your pull request and welcome to the Trino community. We require contributors to sign our Contributor License Agreement, and we don't seem to have you on file. Continue to work with us on the review and improvements in this PR, and submit the signed CLA to cla@trino.io. Photos, scans, or digitally-signed PDF files are all suitable. Processing may take a few days. The CLA needs to be on file before we merge your changes. For more information, see https://github.com/trinodb/cla

cla-bot · 2024-03-28T21:33:31Z

Thank you for your pull request and welcome to the Trino community. We require contributors to sign our Contributor License Agreement, and we don't seem to have you on file. Continue to work with us on the review and improvements in this PR, and submit the signed CLA to cla@trino.io. Photos, scans, or digitally-signed PDF files are all suitable. Processing may take a few days. The CLA needs to be on file before we merge your changes. For more information, see https://github.com/trinodb/cla

lib/trino-filesystem-s3/src/main/java/io/trino/filesystem/s3/S3FileIterator.java

plugin/trino-hive/src/main/java/io/trino/plugin/hive/fs/TrinoFileStatusRemoteIterator.java

plugin/trino-hive/src/main/java/io/trino/plugin/hive/S3StorageClassFilter.java

plugin/trino-hive/src/main/java/io/trino/plugin/hive/HiveConfig.java

pettyjamesm

Minor comments, otherwise LGTM

plugin/trino-hive/src/main/java/io/trino/plugin/hive/S3StorageClassFilter.java

plugin/trino-hive/src/main/java/io/trino/plugin/hive/fs/TrinoFileStatusRemoteIterator.java

plugin/trino-hive/src/test/java/io/trino/plugin/hive/fs/FileSystemDirectoryLister.java

pettyjamesm

Changes LGTM, the test failure appears to be caused by #20326.

plugin/trino-hive/src/main/java/io/trino/plugin/hive/HiveConfig.java

findinpath · 2024-04-10T07:37:12Z

plugin/trino-hive/src/main/java/io/trino/plugin/hive/HiveConfig.java

+
+    @Config("hive.s3.storage-class-filter")
+    public HiveConfig setS3StorageClassFilter(S3StorageClassFilter s3StorageClassFilter)
+    {


Add this configuration in HiveS3Config along with the other hive.s3.* properties.

Per this comment on the previous PR, I believe that HiveConfig is the correct place to put it. Additionally, the trino-hive module (where HiveConfig lies) cannot access the trino-hdfs module (where HiveS3Config lies) directly

lib/trino-filesystem-s3/src/test/java/io/trino/filesystem/s3/TestS3FileSystemAwsS3.java

plugin/trino-hive/src/test/java/io/trino/plugin/hive/TestBackgroundHiveSplitLoader.java

plugin/trino-hive/src/main/java/io/trino/plugin/hive/S3StorageClassFilter.java

pettyjamesm · 2024-04-11T22:17:08Z

/test-with-secrets sha=47508e987aa6c4367698854bffdfc2293bd74e3c

github-actions · 2024-04-11T23:22:15Z

The CI workflow run with tests that require additional secrets has been started: https://github.com/trinodb/trino/actions/runs/8654423316

plugin/trino-hive/src/main/java/io/trino/plugin/hive/S3StorageClassFilter.java

plugin/trino-hive/src/main/java/io/trino/plugin/hive/fs/CachingDirectoryLister.java

plugin/trino-hive/src/test/java/io/trino/plugin/hive/TestBackgroundHiveSplitLoader.java

findinpath

Almost there :)

lib/trino-filesystem-s3/src/main/java/io/trino/filesystem/s3/S3FileIterator.java

anusudarsan · 2024-04-12T21:27:09Z

lib/trino-filesystem-s3/src/test/java/io/trino/filesystem/s3/TestS3FileSystemAwsS3.java

+    void testS3FileIteratorFileEntryTags()
+            throws IOException
+    {
+        try (S3Client s3Client = createS3Client()) {


add a test for the restored object as well?

I believe testing for restored glacier object may be a little difficult since glacier restores are extremely slow. Testing glacier objects is more intuitive as we can mock an s3 bucket with a glacier object; however, in the putobject request there is no way we can manipulate the restore status

pettyjamesm · 2024-04-23T15:07:09Z

Merged, thanks @bangtim!

github-actions bot added the tests:hive label Mar 19, 2024

bangtim marked this pull request as ready for review March 19, 2024 17:04

bangtim marked this pull request as draft March 19, 2024 17:51

ericlgoodman reviewed Mar 19, 2024

View reviewed changes

lib/trino-filesystem-s3/src/main/java/io/trino/filesystem/s3/S3ObjectStorageClassFilter.java Outdated Show resolved Hide resolved

ericlgoodman reviewed Mar 19, 2024

View reviewed changes

lib/trino-filesystem-s3/src/main/java/io/trino/filesystem/s3/S3ObjectStorageClassFilter.java Outdated Show resolved Hide resolved

ericlgoodman reviewed Mar 19, 2024

View reviewed changes

lib/trino-filesystem-s3/src/main/java/io/trino/filesystem/s3/S3ObjectStorageClassFilter.java Outdated Show resolved Hide resolved

bangtim force-pushed the adding-glacier-skipping-logic branch from 2e75199 to 874fcd0 Compare March 28, 2024 17:38

bangtim force-pushed the adding-glacier-skipping-logic branch from 874fcd0 to 3d6e7df Compare March 28, 2024 18:15

bangtim force-pushed the adding-glacier-skipping-logic branch from 3d6e7df to 56a50ca Compare March 28, 2024 18:16

bangtim force-pushed the adding-glacier-skipping-logic branch from 56a50ca to 4435227 Compare March 28, 2024 18:21

bangtim force-pushed the adding-glacier-skipping-logic branch from 4435227 to 45850ca Compare March 28, 2024 18:25

github-actions bot added the hive Hive connector label Mar 28, 2024

bangtim force-pushed the adding-glacier-skipping-logic branch from 45850ca to 1b5a155 Compare March 28, 2024 20:36

bangtim force-pushed the adding-glacier-skipping-logic branch from 1b5a155 to bd5cbaa Compare March 28, 2024 21:33

bangtim changed the title ~~adding glacier skipping logic to s3filesystem~~ adding glacier skipping logic Mar 28, 2024

bangtim changed the title ~~adding glacier skipping logic~~ Add glacier skipping logic Apr 5, 2024

bangtim force-pushed the adding-glacier-skipping-logic branch from 9ae2cf2 to abd0130 Compare April 5, 2024 14:00

pettyjamesm reviewed Apr 5, 2024

View reviewed changes

bangtim force-pushed the adding-glacier-skipping-logic branch from abd0130 to aca6c00 Compare April 8, 2024 19:35

pettyjamesm reviewed Apr 8, 2024

View reviewed changes

plugin/trino-hive/src/main/java/io/trino/plugin/hive/S3StorageClassFilter.java Outdated Show resolved Hide resolved

plugin/trino-hive/src/main/java/io/trino/plugin/hive/fs/TrinoFileStatusRemoteIterator.java Outdated Show resolved Hide resolved

bangtim force-pushed the adding-glacier-skipping-logic branch from aca6c00 to 32b7d9d Compare April 8, 2024 20:04

pettyjamesm reviewed Apr 8, 2024

View reviewed changes

bangtim force-pushed the adding-glacier-skipping-logic branch from 32b7d9d to fec1cb4 Compare April 8, 2024 20:40

pettyjamesm approved these changes Apr 9, 2024

View reviewed changes

findinpath reviewed Apr 10, 2024

View reviewed changes

findinpath requested review from nineinchnick and anusudarsan April 10, 2024 14:51

bangtim force-pushed the adding-glacier-skipping-logic branch from fec1cb4 to 47508e9 Compare April 11, 2024 20:17

github-actions bot added the docs label Apr 11, 2024

findinpath reviewed Apr 12, 2024

View reviewed changes

plugin/trino-hive/src/main/java/io/trino/plugin/hive/S3StorageClassFilter.java Outdated Show resolved Hide resolved

findinpath reviewed Apr 12, 2024

View reviewed changes

plugin/trino-hive/src/main/java/io/trino/plugin/hive/fs/CachingDirectoryLister.java Outdated Show resolved Hide resolved

findinpath reviewed Apr 12, 2024

View reviewed changes

plugin/trino-hive/src/test/java/io/trino/plugin/hive/TestBackgroundHiveSplitLoader.java Outdated Show resolved Hide resolved

findinpath reviewed Apr 12, 2024

View reviewed changes

anusudarsan reviewed Apr 12, 2024

View reviewed changes

Add glacier skipping logic

1695cdf

bangtim force-pushed the adding-glacier-skipping-logic branch from 47508e9 to 1695cdf Compare April 17, 2024 00:40

findepi removed the tests:hive label Apr 18, 2024

pettyjamesm merged commit 9962596 into trinodb:master Apr 23, 2024
68 checks passed

github-actions bot added this to the 446 milestone Apr 23, 2024

bangtim deleted the adding-glacier-skipping-logic branch April 25, 2024 19:16

bangtim restored the adding-glacier-skipping-logic branch April 25, 2024 19:16

colebow mentioned this pull request Apr 29, 2024

Add Trino 446 release notes #21753

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add glacier skipping logic #21164

Add glacier skipping logic #21164

bangtim commented Mar 19, 2024 •

edited

cla-bot bot commented Mar 19, 2024

cla-bot bot commented Mar 19, 2024

cla-bot bot commented Mar 19, 2024

cla-bot bot commented Mar 19, 2024

cla-bot bot commented Mar 21, 2024

cla-bot bot commented Mar 21, 2024

electrum commented Mar 22, 2024

cla-bot bot commented Mar 28, 2024

cla-bot bot commented Mar 28, 2024

cla-bot bot commented Mar 28, 2024

cla-bot bot commented Mar 28, 2024

cla-bot bot commented Mar 28, 2024

cla-bot bot commented Mar 28, 2024

cla-bot bot commented Mar 28, 2024

cla-bot bot commented Mar 28, 2024

pettyjamesm left a comment

pettyjamesm left a comment

findinpath Apr 10, 2024

bangtim Apr 11, 2024

pettyjamesm commented Apr 11, 2024

github-actions bot commented Apr 11, 2024

findinpath left a comment

anusudarsan Apr 12, 2024

bangtim Apr 17, 2024

pettyjamesm commented Apr 23, 2024

Add glacier skipping logic #21164

Add glacier skipping logic #21164

Conversation

bangtim commented Mar 19, 2024 • edited

Description

Additional context and related issues

Release notes

cla-bot bot commented Mar 19, 2024

cla-bot bot commented Mar 19, 2024

cla-bot bot commented Mar 19, 2024

cla-bot bot commented Mar 19, 2024

cla-bot bot commented Mar 21, 2024

cla-bot bot commented Mar 21, 2024

electrum commented Mar 22, 2024

cla-bot bot commented Mar 28, 2024

cla-bot bot commented Mar 28, 2024

cla-bot bot commented Mar 28, 2024

cla-bot bot commented Mar 28, 2024

cla-bot bot commented Mar 28, 2024

cla-bot bot commented Mar 28, 2024

cla-bot bot commented Mar 28, 2024

cla-bot bot commented Mar 28, 2024

pettyjamesm left a comment

Choose a reason for hiding this comment

pettyjamesm left a comment

Choose a reason for hiding this comment

findinpath Apr 10, 2024

Choose a reason for hiding this comment

bangtim Apr 11, 2024

Choose a reason for hiding this comment

pettyjamesm commented Apr 11, 2024

github-actions bot commented Apr 11, 2024

findinpath left a comment

Choose a reason for hiding this comment

anusudarsan Apr 12, 2024

Choose a reason for hiding this comment

bangtim Apr 17, 2024

Choose a reason for hiding this comment

pettyjamesm commented Apr 23, 2024

bangtim commented Mar 19, 2024 •

edited