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.

Sign up for GitHub

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

Jump to bottom

quincy: librados: aio operate functions can set mtimes #52118

Merged
merged 5 commits into from Jun 26, 2023
Merged

quincy: librados: aio operate functions can set mtimes #52118

merged 5 commits into from Jun 26, 2023

Conversation

cbodley
Copy link
Contributor

@cbodley cbodley commented Jun 19, 2023

backport tracker: https://tracker.ceph.com/issues/61731

backport of #51681
parent tracker: https://tracker.ceph.com/issues/61349

this backport was staged using ceph-backport.sh version 16.0.0.6848
find the latest version at https://github.com/ceph/ceph/blob/main/src/script/ceph-backport.sh

@cbodley
librados: aio_operate() supports ObjectWriteOperation::mtime2()
1f4152d 
copy the optional mtime logic from operate(), so that mtimes set on the
ObjectOperation are propagated through aio_operate() as well

Fixes: https://tracker.ceph.com/issues/61349

Signed-off-by: Casey Bodley <cbodley@redhat.com>
(cherry picked from commit cdf54ff)
@cbodley
librados: use ObjectOperationImpl for rados_write_op_t
2681680 
the c++ api uses ObjectOperationImpl to wrap ObjectOperation with
additional storage for an optional mtime. the c api now reuses
ObjectOperationImpl for its write operations only - the mtime isn't
needed for read ops

Signed-off-by: Casey Bodley <cbodley@redhat.com>
(cherry picked from commit 4c8f694)
@cbodley
librados: rados_aio_write_op_operate() propagates mtime
1634a07 
Signed-off-by: Casey Bodley <cbodley@redhat.com>
(cherry picked from commit 0bfb144)
@cbodley
librados: rados_write_op_operate() uses ObjectOperationImpl for mtime
003a49e 
Signed-off-by: Casey Bodley <cbodley@redhat.com>
(cherry picked from commit b888821)
@cbodley cbodley requested review from a team as code owners June 19, 2023 18:33
@cbodley cbodley added this to the quincy milestone Jun 19, 2023
@yuriw
Copy link
Contributor

yuriw commented Jun 22, 2023

@cbodley I can't build branch with this PR and suspect this PR

Pls see https://jenkins.ceph.com/job/ceph-dev-new-build/ARCH=x86_64,AVAILABLE_ARCH=x86_64,AVAILABLE_DIST=centos8,DIST=centos8,MACHINE_SIZE=gigantic/70756//consoleFull

[ 63%] Building CXX object src/test/librados/CMakeFiles/ceph_test_rados_api_aio.dir/aio.cc.o
In file included from /home/jenkins-build/build/workspace/ceph-dev-new-build/ARCH/x86_64/AVAILABLE_ARCH/x86_64/AVAILABLE_DIST/centos8/DIST/centos8/MACHINE_SIZE/gigantic/release/17.2.6-729-ga19861cc/rpm/el8/BUILD/ceph-17.2.6-729-ga19861cc/src/googletest/googletest/include/gtest/gtest.h:387,
from /home/jenkins-build/build/workspace/ceph-dev-new-build/ARCH/x86_64/AVAILABLE_ARCH/x86_64/AVAILABLE_DIST/centos8/DIST/centos8/MACHINE_SIZE/gigantic/release/17.2.6-729-ga19861cc/rpm/el8/BUILD/ceph-17.2.6-729-ga19861cc/src/test/librados/aio.cc:17:
/home/jenkins-build/build/workspace/ceph-dev-new-build/ARCH/x86_64/AVAILABLE_ARCH/x86_64/AVAILABLE_DIST/centos8/DIST/centos8/MACHINE_SIZE/gigantic/release/17.2.6-729-ga19861cc/rpm/el8/BUILD/ceph-17.2.6-729-ga19861cc/src/test/librados/aio.cc: In member function ‘virtual void LibRadosAio_OperateMtime_Test::TestBody()’:
/home/jenkins-build/build/workspace/ceph-dev-new-build/ARCH/x86_64/AVAILABLE_ARCH/x86_64/AVAILABLE_DIST/centos8/DIST/centos8/MACHINE_SIZE/gigantic/release/17.2.6-729-ga19861cc/rpm/el8/BUILD/ceph-17.2.6-729-ga19861cc/src/test/librados/aio.cc:802:18: error: ‘rados_stat2’ was not declared in this scope; did you mean ‘rados_stat’?
802 | ASSERT_EQ(0, rados_stat2(test_data.m_ioctx, "foo", &size, &mtime));
| ^~~~~~~~~~~
/home/jenkins-build/build/workspace/ceph-dev-new-build/ARCH/x86_64/AVAILABLE_ARCH/x86_64/AVAILABLE_DIST/centos8/DIST/centos8/MACHINE_SIZE/gigantic/release/17.2.6-729-ga19861cc/rpm/el8/BUILD/ceph-17.2.6-729-ga19861cc/src/test/librados/aio.cc: In member function ‘virtual void LibRadosAio_Operate2Mtime_Test::TestBody()’:
/home/jenkins-build/build/workspace/ceph-dev-new-build/ARCH/x86_64/AVAILABLE_ARCH/x86_64/AVAILABLE_DIST/centos8/DIST/centos8/MACHINE_SIZE/gigantic/release/17.2.6-729-ga19861cc/rpm/el8/BUILD/ceph-17.2.6-729-ga19861cc/src/test/librados/aio.cc:833:18: error: ‘rados_stat2’ was not declared in this scope; did you mean ‘rados_stat’?
833 | ASSERT_EQ(0, rados_stat2(test_data.m_ioctx, "foo", &size, &mtime));
| ^~~~~~~~~~~
make[2]: *** [src/test/librados/CMakeFiles/ceph_test_rados_api_aio.dir/build.make:76: src/test/librados/CMakeFiles/ceph_test_rados_api_aio.dir/aio.cc.o] Error 1
make[1]: *** [CMakeFiles/Makefile2:14879: src/test/librados/CMakeFiles/ceph_test_rados_api_aio.dir/all] Error 2
make[1]: *** Waiting for unfinished jobs....

@cbodley
Copy link
Contributor Author

cbodley commented Jun 22, 2023

@cbodley I can't build branch with this PR and suspect this PR

okay, looking

@cbodley
Copy link
Contributor Author

cbodley commented Jun 22, 2023

error: ‘rados_stat2’ was not declared in this scope; did you mean ‘rados_stat’?

ok, i see that rados_stat2() was added to the C API in #46823 after quincy, and wasn't backported

my original PR #51681 added rados_aio_write_op_operate2() to the C API, and its unit test relies on rados_stat2() to read back the high-resolution timestamp. that new function was not required to fix the bug in https://tracker.ceph.com/issues/61349, though, so i'll remove that commit from the backport along with its problematic test case. i'll do the same for the pacific backport

cc @rzarzynski @idryomov

@cbodley
test/librados: test ability to set mtimes with aio_operate
855697d 
Signed-off-by: Casey Bodley <cbodley@redhat.com>
(cherry picked from commit ec29234)

Conflicts:
	src/test/librados/aio.cc:
	  removed test case for rados_aio_write_op_operate2()
which wasn't backported
	  test case for rados_aio_write_op_operate() uses rados_stat()
instead of rados_stat2() which doesn't exist on quincy
@cbodley cbodley mentioned this pull request Jun 22, 2023
Merged
@cbodley
Copy link
Contributor Author

cbodley commented Jun 22, 2023

updated, conflicts are documented in the final commit

@ljflores
Copy link
Contributor

Rados suite review: https://tracker.ceph.com/projects/rados/wiki/QUINCY#httpstrellocomciQLrAz6r1791-wip-yuri6-testing-2023-06-22-1005-quincy-old-wip-yuri6-testing-2023-06-22-0827-quincy

@yuriw
Copy link
Contributor

yuriw commented Jun 23, 2023

jenkins test make check

@yuriw yuriw merged commit 8ff10a0 into ceph:quincy Jun 26, 2023
10 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@idryomov idryomov idryomov approved these changes

@ljflores ljflores ljflores approved these changes

Assignees
No one assigned
Labels
core needs-qa rbd tests wip-yuri6-testing
Projects
None yet
Milestone
quincy
4 participants
@cbodley @yuriw @ljflores @idryomov