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

NSFS | versioning | copy_object - close chunkfs read stream to prevent stream being closed after stat #8526

Merged
merged 1 commit into from
Nov 25, 2024

Conversation

nadavMiz
Copy link
Contributor

@nadavMiz nadavMiz commented Nov 13, 2024

Explain the changes

  1. chunkfs is a Transform stream, however read_object_stream expects a write stream, so it only closes the write part. so the stream itself is still alive. it is later closed automatically (probably by garbage collector?), but this may happen after we call stat on the written file, so the mtime we stat is later modified when the stream is closed. this causes the mtime of the file to be different then that of the version. on GPFS we will not notice this until we call safe_move_posix that validates the inode and mtime of the file with the version, causing it to fail.

Issues: Fixed #8469

Testing Instructions:

  1. run NC_CORETEST=true GPFS_ROOT_PATH=/ibm/fs1/tmp GPFS_DL_PATH="/usr/lpp/mmfs/lib/libgpfs.so" node ./node_modules/mocha/bin/mocha src/test/unit_tests/test_bucketspace_versioning.js on GPFS machine
  2. ceph test: test_versioning_obj_suspended_copy
  • Doc added/updated
  • Tests added

@nadavMiz nadavMiz changed the title NSFS | versioning | close chunkfs read stream to prevent stream being closed after stat NSFS | versioning | copy_object - close chunkfs read stream to prevent stream being closed after stat Nov 13, 2024
@nadavMiz nadavMiz requested a review from romayalon November 14, 2024 13:12
@romayalon
Copy link
Contributor

@nadavMiz and I had a discussion about ChunkFS being a Transform stream which is also a readable stream, this is causing us a lot of issues and redundant, per Nadav, @guymguym wrote some chunkFS code that omits the readable stream part, we should re-consider this solution again.

@romayalon
Copy link
Contributor

Per our discussion -

  1. We saw that when reproduced, the readable stream never gett closed and the writable stream get closed after the stat(), therefore the mtime changed after the close of the write stream, this caused the distinction between the mtime on the xattr and on the file. When closing the readable stream (using resume() we saw that the writeable stream is getting closed before the stat().
  2. Please add the full explantation of this behavior above the resume().
  3. If possible please add a dbg message when chunkfs is getting closed for follow-up.
    Approving.

@nadavMiz nadavMiz force-pushed the copy-object-close-read branch from cf76add to 6d2f617 Compare November 25, 2024 09:58
@nadavMiz nadavMiz closed this Nov 25, 2024
@nadavMiz nadavMiz force-pushed the copy-object-close-read branch from 6d2f617 to 412e804 Compare November 25, 2024 09:59
@nadavMiz nadavMiz reopened this Nov 25, 2024
…t stream being closed after stat

Signed-off-by: nadav mizrahi <[email protected]>
@nadavMiz nadavMiz force-pushed the copy-object-close-read branch from c15af6e to cbf7c69 Compare November 25, 2024 10:15
@nadavMiz nadavMiz merged commit e5e6a35 into noobaa:master Nov 25, 2024
10 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
2 participants