Try Getting Time from Server for List Calls #567
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
BrentSouza
requested review from
ilkinulas and
seruman
and removed request for
a team
|
I built this locally and it did now successfully return the future-dated (rounded up to the next whole second) object in our environment. |
ilkinulas
reviewed
Jul 1, 2023
| // track the instant object iteration began, | ||
| // so it can be used to bypass objects created after this instant | ||
| if now.IsZero() { | ||
| now = time.Now().UTC() | ||
| if serverDate != "" { | ||
| n, err := http.ParseTime(serverDate) |
There was a problem hiding this comment.
http.ParseTime function is not able to accurately represent the last modified time of an S3 object, because the latter has a much higher precision. This is causing the tests to fail.
For example:
- Parsed time in header : 2023-07-01 19:43:58 +0000 UTC
- Last Modified time of S3 object: 2023-07-01 19:43:58.058 +0000 UTC
I will let you know if I am able to find a solution to the time comparison issue.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
We have noticed that
s5cmd lssometimes fails to list objects immediately after they are written. This seems to be related to clock skew sinces5cmdgets the current time from the client and compares it against each object's lastModified date. This is detailed a bit in issue #531In our case, S3 servers seem to be a couple seconds ahead of our servers (which are synced with NTP servers so not sure where the issue lies). While troubleshooting, we did notice that the Date header from S3's http responses lined up with the future times we saw in the lastModified metadata.
This patch will request the Date header from the response and use it for the timestamp. It will fallback to the client's current time if the header is not present or in the event that the date cannot be parsed.
I've modified the tests to check for both scenarios and locally all unit test are working. Please let me know if there are any additional checks you'd like me to run.