Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fixed marker being saved under wrong key in marker cache #531

Merged
merged 1 commit into from
Jul 10, 2023
Merged

fixed marker being saved under wrong key in marker cache #531

merged 1 commit into from
Jul 10, 2023

Conversation

Decard6
Copy link
Contributor

@Decard6 Decard6 commented Jul 5, 2023

Currently pagination is broken for backends that use opaque markers. It is so because the key (last key from the set) under which a real marker is saved is different than the key (encoded marker) that S3Proxy is trying to use to read a real marker.

It results with S3Proxy being able to fetch only 1 page:

2023-07-04 14:02:31         13 hw
2023-07-04 14:37:11         13 hw-a-0
2023-07-04 14:43:21         13 hw-aa-0
2023-07-04 14:43:22         13 hw-aa-1
2023-07-04 14:43:29         13 hw-aa-10
2023-07-04 14:43:59         13 hw-aa-100
2023-07-04 14:48:29         13 hw-aa-1000
2023-07-04 14:48:30         13 hw-aa-1001
2023-07-04 14:48:30         13 hw-aa-1002
2023-07-04 14:48:30         13 hw-aa-1003

An error occurred (BadDigest) when calling the ListObjectsV2 operation (reached max retries: 4): Bad Request

This could be solved by either using last key from the set as a marker in response or using encoded nextMarker as part of key in cache. In this PR I decided to implement the second solution.

@gaul gaul merged commit cf4db28 into gaul:master Jul 10, 2023
@gaul
Copy link
Owner

gaul commented Jul 10, 2023

Thank you for your contribution @Decard6!

@Decard6
Copy link
Contributor Author

Decard6 commented Jul 10, 2023

@gaul You're welcome. Do you plan to release 2.0.1?

@Decard6 Decard6 deleted the bugfix/opaque-markers-cache branch July 11, 2023 07:51
@larshagencognite
Copy link
Contributor

@Decard6 @gaul This change broke clients that use list blobs v1, and use the last name in the response as the next marker. I don't think this is a great way to iterate over objects, but we are using a third party client that iterates in this way. What is the point of the cache after this change?

@larshagencognite
Copy link
Contributor

For V1 we should keep the old code, and for V2 we should probably handle the encoding/decoding in a different way, as it is probably not related to whether the provider uses opaque markers in V1.

From S3 list blobs v2 docs:

NextContinuationToken
NextContinuationToken is sent when isTruncated is true, which means there are more keys in the bucket that can be listed. The next list requests to Amazon S3 can be continued with this NextContinuationToken. NextContinuationToken is obfuscated and is not a real key
Type: String

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants