NSFS | versioning | calculate multipart upload version by creation time #8644
Conversation
nadavMiz
force-pushed
the
multipart-version-ordering
branch
6 times, most recently
from
12445b6 to
2c10bc0
Compare
nadavMiz
force-pushed
the
multipart-version-ordering
branch
from
2c10bc0 to
b3b9a42
Compare
|
@nadavMiz I'm adding a TODO here so we will not forget - run the test on MAC as well. |
nadavMiz
force-pushed
the
multipart-version-ordering
branch
2 times, most recently
from
cc844df to
7d5bf69
Compare
| const create_object_upload_stat = await nb_native().fs.stat(fs_context, create_object_upload_path); | ||
| //according to aws, multipart upload version time should be calculated based on creation rather then completion time. | ||
| //change the files mtime to match the creation time | ||
| await nb_native().fs.utimensat(fs_context, upload_path, {modtime: create_object_upload_stat.mtimeNsBigint}); |
There was a problem hiding this comment.
do you think we should fail the whole upload in case we were not able to set the mtime?
There was a problem hiding this comment.
I guess we can use the existing time in case it fails
There was a problem hiding this comment.
@guymguym WDYT? I'm not sure, since it might create a wrong .versions/ stack order, but on the other hand we probably don't want to fail on that
src/native/fs/fs_napi.cpp
Outdated
| SYSCALL_OR_RETURN(utimensat(AT_FDCWD, _path.c_str(), _times, 0)); | ||
| } | ||
|
|
||
| void get_time_from_info(const Napi::Object& napi_times, const std::string& time_type, struct timespec& timeval, std::string& log_str) { |
There was a problem hiding this comment.
pls move helper functions to not be in the middle of the workers
There was a problem hiding this comment.
get_time_from_info is only relevant to utimensat, so I added it as a member function to the class. where would you propose to move it?
There was a problem hiding this comment.
I saw helper functions both at the top and at the bottom of this file, so check what you think is the best location
nadavMiz
force-pushed
the
multipart-version-ordering
branch
2 times, most recently
from
8c7187b to
5bcf21d
Compare
Signed-off-by: nadav mizrahi <[email protected]>
nadavMiz
force-pushed
the
multipart-version-ordering
branch
from
5bcf21d to
ba6cf8c
Compare
Explain the changes
according to aws specifications multipart upload version creation time rather than completion time. this effects version ordering as will no have the latest version if another object was uploaded after multipart upload creation but before completion
utimensatto change access time of files. see https://man7.org/linux/man-pages/man3/futimens.3p.htmlIssues: Fixed #8411
Testing Instructions:
sudo node ./node_modules/mocha/bin/mocha src/test/unit_tests/test_nb_native_fs.jssudo NC_CORETEST=true node ./node_modules/mocha/bin/mocha src/test/unit_tests/test_bucketspace_versioning.js