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

rgw: add support for the BulkUpload of Swift API #12243

Merged
merged 11 commits into from Apr 3, 2017

Conversation

Projects
None yet
6 participants
@rzarzynski
Contributor

rzarzynski commented Nov 30, 2016

No description provided.

@oritwas

oritwas approved these changes Dec 1, 2016

great!

@@ -446,7 +446,9 @@ enum RGWOpType {
RGW_OP_GET_INFO,
/* rgw specific */
RGW_OP_ADMIN_SET_METADATA
RGW_OP_ADMIN_SET_METADATA,

This comment has been minimized.

@oritwas

oritwas Dec 1, 2016

Contributor

nit: empty line

This comment has been minimized.

@rzarzynski

rzarzynski Dec 9, 2016

Contributor

FIXED.

@oritwas

This comment has been minimized.

Contributor

oritwas commented Dec 1, 2016

@rzarzynski , can this be tested as part of swift test?

@rzarzynski

This comment has been minimized.

Contributor

rzarzynski commented Dec 1, 2016

@oritwas: I went through the functional tests of Swift we use but found nothing related.

Fortunately, Tempest has 3 tests for verifying the bulk operations. I'm posting results below:

$ ./run_tempest.sh -V tempest.api.object_storage.test_account_bulk.BulkTest
WARNING: This script is deprecated and will be removed in the near future. Please migrate to tempest run or another method of launching a test runner
running=OS_STDOUT_CAPTURE=${OS_STDOUT_CAPTURE:-1} \
OS_STDERR_CAPTURE=${OS_STDERR_CAPTURE:-1} \
OS_TEST_TIMEOUT=${OS_TEST_TIMEOUT:-500} \
OS_TEST_LOCK_PATH=${OS_TEST_LOCK_PATH:-${TMPDIR:-'/tmp'}} \
${PYTHON:-python} -m subunit.run discover -t ${OS_TOP_LEVEL:-./} ${OS_TEST_PATH:-./tempest/test_discover} --list 
running=OS_STDOUT_CAPTURE=${OS_STDOUT_CAPTURE:-1} \
OS_STDERR_CAPTURE=${OS_STDERR_CAPTURE:-1} \
OS_TEST_TIMEOUT=${OS_TEST_TIMEOUT:-500} \
OS_TEST_LOCK_PATH=${OS_TEST_LOCK_PATH:-${TMPDIR:-'/tmp'}} \
${PYTHON:-python} -m subunit.run discover -t ${OS_TOP_LEVEL:-./} ${OS_TEST_PATH:-./tempest/test_discover}  --load-list /tmp/tmplrDWSg
{0} tempest.api.object_storage.test_account_bulk.BulkTest.test_bulk_delete [0.261942s] ... ok
{0} tempest.api.object_storage.test_account_bulk.BulkTest.test_bulk_delete_by_POST [0.171467s] ... ok
{0} tempest.api.object_storage.test_account_bulk.BulkTest.test_extract_archive [0.100945s] ... ok

======
Totals
======
Ran: 3 tests in 3.0000 sec.
 - Passed: 3
 - Skipped: 0
 - Expected Fail: 0
 - Unexpected Success: 0
 - Failed: 0
Sum of execute time for each test: 0.5344 sec.

==============
Worker Balance
==============
 - Worker 0 (3 tests) => 0:00:00.536726
adam@adam-VirtualBox:/work/tempest$ ./run_tempest.sh -V tempest.api.object_storage.test_account_bulk.BulkTest
WARNING: This script is deprecated and will be removed in the near future. Please migrate to tempest run or another method of launching a test runner
running=OS_STDOUT_CAPTURE=${OS_STDOUT_CAPTURE:-1} \
OS_STDERR_CAPTURE=${OS_STDERR_CAPTURE:-1} \
OS_TEST_TIMEOUT=${OS_TEST_TIMEOUT:-500} \
OS_TEST_LOCK_PATH=${OS_TEST_LOCK_PATH:-${TMPDIR:-'/tmp'}} \
${PYTHON:-python} -m subunit.run discover -t ${OS_TOP_LEVEL:-./} ${OS_TEST_PATH:-./tempest/test_discover} --list 
running=OS_STDOUT_CAPTURE=${OS_STDOUT_CAPTURE:-1} \
OS_STDERR_CAPTURE=${OS_STDERR_CAPTURE:-1} \
OS_TEST_TIMEOUT=${OS_TEST_TIMEOUT:-500} \
OS_TEST_LOCK_PATH=${OS_TEST_LOCK_PATH:-${TMPDIR:-'/tmp'}} \
${PYTHON:-python} -m subunit.run discover -t ${OS_TOP_LEVEL:-./} ${OS_TEST_PATH:-./tempest/test_discover}  --load-list /tmp/tmpwVJAGd
{0} tempest.api.object_storage.test_account_bulk.BulkTest.test_bulk_delete [6.239440s] ... ok
{0} tempest.api.object_storage.test_account_bulk.BulkTest.test_bulk_delete_by_POST [0.139734s] ... ok
{0} tempest.api.object_storage.test_account_bulk.BulkTest.test_extract_archive [0.089281s] ... ok

======
Totals
======
Ran: 3 tests in 10.0000 sec.
 - Passed: 3
 - Skipped: 0
 - Expected Fail: 0
 - Unexpected Success: 0
 - Failed: 0
Sum of execute time for each test: 6.4685 sec.

==============
Worker Balance
==============
 - Worker 0 (3 tests) => 0:00:06.471241

Tested using relatively fresh Tempest (Wed Nov 16 10:13:27 2016 +0000, 51feb121085cf0a4d70af4b2b086f03991d5bceb).

@mattbenjamin

This comment has been minimized.

Contributor

mattbenjamin commented Dec 8, 2016

@rzarzynski can you fix the following build failure?

[ 14%] Building CXX object src/rgw/CMakeFiles/rgw_a.dir/rgw_op.cc.o
/home/mbenjamin/ceph-noob/src/rgw/rgw_op.cc: In member function \u2018int RGWBulkUploadOp::handle_file(boost::string_ref, size_t, RGWBulkUploadOp::AlignedStreamGetter&)\u2019:
/home/mbenjamin/ceph-noob/src/rgw/rgw_op.cc:5781:49: error: \u2018struct md_config_t\u2019 has no member named \u2018rgw_compression_type\u2019
const auto& compression_type = s->cct->_conf->rgw_compression_type;
^
src/rgw/CMakeFiles/rgw_a.dir/build.make:1094: recipe for target 'src/rgw/CMakeFiles/rgw_a.dir/rgw_op.cc.o' failed
make[2]: *** [src/rgw/CMakeFiles/rgw_a.dir/rgw_op.cc.o] Error 1
CMakeFiles/Makefile2:20709: recipe for target 'src/rgw/CMakeFiles/rgw_a.dir/all' failed
make[1]: *** [src/rgw/CMakeFiles/rgw_a.dir/all] Error 2
Makefile:138: recipe for target 'all' failed
make: *** [all] Error 2

int RGWBulkUploadOp::handle_dir_verify_permission()
{
if (s->user->max_buckets > 0) {

This comment has been minimized.

@rzarzynski

rzarzynski Dec 9, 2016

Contributor

Switched from:

if (s->user->max_buckets) {
return op_ret;
}
if (buckets.count() >= static_cast<size_t>(s->user->max_buckets)) {

This comment has been minimized.

@rzarzynski

rzarzynski Dec 9, 2016

Contributor

Switched from:

    if ((int)buckets.count() >= s->user->max_buckets) {
@rzarzynski

This comment has been minimized.

Contributor

rzarzynski commented Dec 9, 2016

@oritwas, @mattbenjamin: rebased to today's master, fixed the build error related torgw_compression_type as well as indentation in few places.

@idealguo

This comment has been minimized.

idealguo commented Apr 1, 2017

Hi , we need this feature to accelerate migration of data, is it possible in luminous?

@oritwas

This comment has been minimized.

Contributor

oritwas commented Apr 1, 2017

@rzarzynski , can you rebase?

rzarzynski added some commits Nov 21, 2016

rgw: initial class structure for BulkUpload of Swift API.
Signed-off-by: Radoslaw Zarzynski <rzarzynski@mirantis.com>
rgw: implement RGWBulkUploadOp::DecoratedStreamGetter.
Signed-off-by: Radoslaw Zarzynski <rzarzynski@mirantis.com>
rgw: implement the TAR extraction loop of Swift's BulkUpload.
Signed-off-by: Radoslaw Zarzynski <rzarzynski@mirantis.com>
rgw: implement the TAR format interpreter.
Signed-off-by: Radoslaw Zarzynski <rzarzynski@mirantis.com>
@rzarzynski

This comment has been minimized.

Contributor

rzarzynski commented Apr 1, 2017

@oritwas: rebased. There are absolutely no other changes than resolving the trivial merge conflict in rgw_common.h.

@oritwas oritwas added the needs-qa label Apr 1, 2017

@oritwas

This comment has been minimized.

Contributor

oritwas commented Apr 2, 2017

@rzarzynski , it looks like the build failed.
Can you check this please

rzarzynski added some commits Nov 24, 2016

rgw: implement the AlignedStreamGetter.
Signed-off-by: Radoslaw Zarzynski <rzarzynski@mirantis.com>
rgw: implement the basic security check for BulkUpload of Swift API.
Signed-off-by: Radoslaw Zarzynski <rzarzynski@mirantis.com>
@rzarzynski

This comment has been minimized.

Contributor

rzarzynski commented Apr 2, 2017

@oritwas: we had an undetected conflict with the auth rework (auth_identity instead of auth.identity). It's fixed now but I will spawn a Tempest run against the branch.

@rzarzynski rzarzynski changed the title from rgw: add support for the BulkUpload of Swift API to [DNM] rgw: add support for the BulkUpload of Swift API Apr 2, 2017

rzarzynski added some commits Nov 24, 2016

rgw: implement the container creation in BulkUpload of Swift API.
Signed-off-by: Radoslaw Zarzynski <rzarzynski@mirantis.com>
rgw: implement the object creation in BulkUpload of Swift API.
Signed-off-by: Radoslaw Zarzynski <rzarzynski@mirantis.com>
rgw: optimize metadata caching in BulkUpload of Swift API.
Signed-off-by: Radoslaw Zarzynski <rzarzynski@mirantis.com>
rgw: implement the full response generation in BulkUpload of Swift API.
Signed-off-by: Radoslaw Zarzynski <rzarzynski@mirantis.com>
rgw: add the check for Content-Length in BulkUpload.
Signed-off-by: Radoslaw Zarzynski <rzarzynski@mirantis.com>
@rzarzynski

This comment has been minimized.

Contributor

rzarzynski commented Apr 2, 2017

@oritwas: I fixed other, undetected merge conflicts and verified the branch locally with Tempest. It confirmed the BulkUpload is operational:

$ ./run_tempest.sh -V tempest.api.object_storage.test_account_bulk.BulkTest
WARNING: This script is deprecated and will be removed in the near future. Please migrate to tempest run or another method of launching a test runner
running=OS_STDOUT_CAPTURE=${OS_STDOUT_CAPTURE:-1} \
OS_STDERR_CAPTURE=${OS_STDERR_CAPTURE:-1} \
OS_TEST_TIMEOUT=${OS_TEST_TIMEOUT:-500} \
OS_TEST_LOCK_PATH=${OS_TEST_LOCK_PATH:-${TMPDIR:-'/tmp'}} \
${PYTHON:-python} -m subunit.run discover -t ${OS_TOP_LEVEL:-./} ${OS_TEST_PATH:-./tempest/test_discover} --list 
running=OS_STDOUT_CAPTURE=${OS_STDOUT_CAPTURE:-1} \
OS_STDERR_CAPTURE=${OS_STDERR_CAPTURE:-1} \
OS_TEST_TIMEOUT=${OS_TEST_TIMEOUT:-500} \
OS_TEST_LOCK_PATH=${OS_TEST_LOCK_PATH:-${TMPDIR:-'/tmp'}} \
${PYTHON:-python} -m subunit.run discover -t ${OS_TOP_LEVEL:-./} ${OS_TEST_PATH:-./tempest/test_discover}  --load-list /tmp/tmppXAEL4
{0} tempest.api.object_storage.test_account_bulk.BulkTest.test_bulk_delete [3.888314s] ... ok
{0} tempest.api.object_storage.test_account_bulk.BulkTest.test_bulk_delete_by_POST [0.133276s] ... ok
{0} tempest.api.object_storage.test_account_bulk.BulkTest.test_extract_archive [0.085923s] ... ok

======
Totals
======
Ran: 3 tests in 8.0000 sec.
 - Passed: 3
 - Skipped: 0
 - Expected Fail: 0
 - Unexpected Success: 0
 - Failed: 0
Sum of execute time for each test: 4.1075 sec.

==============
Worker Balance
==============
 - Worker 0 (3 tests) => 0:00:04.108543

I've also pushed the branch for gitbuilders and hopefully will schedule a Teuthology run this evening.

@rzarzynski rzarzynski changed the title from [DNM] rgw: add support for the BulkUpload of Swift API to rgw: add support for the BulkUpload of Swift API Apr 2, 2017

@rzarzynski

This comment has been minimized.

Contributor

rzarzynski commented Apr 2, 2017

@rzarzynski

This comment has been minimized.

Contributor

rzarzynski commented Apr 3, 2017

It looks the run on mira got stuck. Rescheduled dead + failed on smithi: http://pulpito.ceph.com/rzarzynski-2017-04-03_08:05:16-rgw-wip-rgw-bulkupload---basic-smithi/.

@oritwas oritwas merged commit 14867ad into ceph:master Apr 3, 2017

3 checks passed

Signed-off-by all commits in this PR are signed
Details
Unmodifed Submodules submodules for project are unmodified
Details
default Build finished.
Details
@wjwithagen

This comment has been minimized.

Contributor

wjwithagen commented Apr 4, 2017

@rzarzynski @oritwas
Hi,

I'm having difficulty getting this PR working on my FreeBSD port.
This could be also due to the way std::initializer_list is declared on FreeBSD, but the folllowing construct errors out:

static constexpr std::initializer_list terminal_errors = {
-EACCES, -EPERM
};

Error:

/home/jenkins/workspace/ceph-master/src/rgw/rgw_op.h:405:47: error: constexpr variable cannot have non-literal type 'const std::initializer_list<int>'
  static constexpr std::initializer_list<int> terminal_errors = {
                                              ^
/usr/include/c++/v1/initializer_list:59:29: note: 'initializer_list<int>' is not literal because it is not an aggregate and has no constexpr constructors other than copy or move constructors
class _LIBCPP_TYPE_VIS_ONLY initializer_list
                            ^
1 error generated.

Which I thought to be easy to fix, but reading up on this, it is sort of a tarpit which is fixed in c++14...

The best I can get current code is deleting both static and constexpr, which then gives:
/home/jenkins/workspace/ceph-master/src/rgw/rgw_op.h:405:48: warning: array backing the initializer list will be destroyed at the end of the constructor [-Wdangling-initializer-list]
std::initializer_list terminal_errors = {
^
1 warning generated.

So perhaps a better solution to initalise separate?

But then again later on a reference to terminal_errors give the next error:

/home/jenkins/workspace/ceph-master/src/rgw/rgw_op.cc:5502:45: error: non-static data member defined out-of-line
std::initializer_list<int> RGWBulkUploadOp::terminal_errors;
                           ~~~~~~~~~~~~~~~~~^

Could you suggest things to try?

Thanx,
--WjW

@rzarzynski

This comment has been minimized.

Contributor

rzarzynski commented Apr 4, 2017

@wjwithagen: thanks for the info! I will try to find a solution today. std::array<int, 2> hopefully might do. Unfortunately, I don't have any FreeBSD-based environment at the moment. :-( Is there a possibility to have your help in the matter of testing/patch verification?

@rzarzynski

This comment has been minimized.

Contributor

rzarzynski commented Apr 4, 2017

@wjwithagen: just created #14314.

@wjwithagen

This comment has been minimized.

Contributor

wjwithagen commented Apr 4, 2017

@rzarzynski
Thanx
I'll see if Clang groks it. But I'm out for the evening, so it could take 'till tomorow,

@wjwithagen

This comment has been minimized.

Contributor

wjwithagen commented Apr 4, 2017

http://cephdev.digiware.nl:8180/jenkins/job/ceph-master/504/display/redirect

Looks like perhaps the static make it very local in Clang?

/home/jenkins/workspace/ceph-master/build/lib/librgw.so: undefined reference to `RGWBulkUploadOp::terminal_errors'
cc: error: linker command failed with exit code 1 (use -v to see invocation)

Link Error: RGW library not found
gmake[2]: *** [src/pybind/rgw/CMakeFiles/cython_rgw.dir/build.make:57: src/pybind/rgw/CMakeFiles/cython_rgw] Error 1
@Jing-Scott

This comment has been minimized.

Contributor

Jing-Scott commented Apr 14, 2017

Hi, @rzarzynski

I've tested the feature locally. It didn't worked as expected. The below is the result.

The result on SWIFT worked well.

  • SWIFT
root@ceph01 jingwenjun]# curl -i -X PUT -T test1.tar http://192.168.141.130:8080/v1/AUTH_test/bulk-con?extract-archive=tar -H 'X-Auth-Token: AUTH_tkaf4be01180e04a6ea6742af3c907cb32'
HTTP/1.1 100 Continue

HTTP/1.1 200 OK
Content-Type: text/plain
X-Trans-Id: txb5c8a5bf5f8e45269e184-0058efb0b5
X-Openstack-Request-Id: txb5c8a5bf5f8e45269e184-0058efb0b5
Date: Thu, 13 Apr 2017 17:09:09 GMT
Transfer-Encoding: chunked

Number Files Created: 2
Response Body: 
Response Status: 201 Created
Errors:

The return code on RadosGW is 202, which is not 200. And there is no object in container bulk-con.

  • RadosGW
[root@CephS debugfile]# curl -i -X PUT -T test2.tar http://127.0.0.1:8000/swift/v1/bulk-con?extract-archive=tar -H 'X-Auth-Token: AUTH_rgwtk0b00000061646d696e3a73776966741c9664f00c5b22bd5a07f1587117c615732ef0336b6f1c18099b41a463f618648d3d203a'
HTTP/1.1 100 CONTINUE

HTTP/1.1 202 Accepted
Content-Length: 0
X-Trans-Id: tx000000000000000000011-0058efb65d-1018-default
Accept-Ranges: bytes
Content-Type: text/plain; charset=utf-8
Date: Thu, 13 Apr 2017 17:33:18 GMT

Can you provide some testing steps or results? Thanks a lot!

@rzarzynski

This comment has been minimized.

Contributor

rzarzynski commented Apr 14, 2017

@Jing-Scott: currently we do support bulk uploads only at the account level and only using non-compressed tars. Though, there is an infrastructure to accommodate those things in the future. For instance:

  /* Create an instance of stream-abstracting class. Having this indirection
   * allows for easy introduction of decompressors like gzip and bzip2. */
  auto stream = create_stream();
  if (! stream) {
    return;
  }

Manual verification:

$ curl -i "${publicURL}/?extract-archive=tar" -X PUT -H "X-Auth-Token: ${token}"  -T bulkupload.tar 
HTTP/1.1 100 CONTINUE

HTTP/1.1 200 OK
Transfer-Encoding: chunked
X-Trans-Id: tx000000000000000000008-0058f08803-1023-default
Content-Type: text/plain; charset=utf-8
Date: Fri, 14 Apr 2017 08:27:48 GMT

Number Files Created: 2
Response Body: 
Response Status: 201 Created
$ curl -i "${publicURL}/" -X GET -H "X-Auth-Token: ${token}"
HTTP/1.1 200 OK
Content-Length: 10
X-Timestamp: 1492158502.47841
X-Account-Container-Count: 1
X-Account-Object-Count: 2
X-Account-Bytes-Used: 43
X-Account-Bytes-Used-Actual: 8192
X-Account-Access-Control: {}
X-Trans-Id: tx000000000000000000009-0058f08826-1023-default
Accept-Ranges: bytes
Content-Type: text/plain; charset=utf-8
Date: Fri, 14 Apr 2017 08:28:22 GMT

bulkupload
$ curl -i "${publicURL}/bulkupload" -X GET -H "X-Auth-Token: ${token}"
HTTP/1.1 200 OK
Content-Length: 9
X-Timestamp: 1492158467.99134
X-Container-Object-Count: 2
X-Container-Bytes-Used: 43
X-Container-Bytes-Used-Actual: 8192
X-Storage-Policy: default-placement
X-Trans-Id: tx00000000000000000000a-0058f0883b-1023-default
Accept-Ranges: bytes
Content-Type: text/plain; charset=utf-8
Date: Fri, 14 Apr 2017 08:28:43 GMT

obj1
obj2

This is how Tempest employs the bulk upload feature:

####
T 127.0.0.1:36229 -> 127.0.0.1:8000 [AP]
PUT /v1/KEY_2987047fc47840058e89e6182e0d96c3/?extract-archive=tar HTTP/1.1.
Host: 127.0.0.1:8000.
Content-Length: 10240.
connection: close.
user-agent: Python-httplib2/0.8 (gzip).
accept-encoding: gzip, deflate.
x-auth-token: 72581465076a40349238c56a9b9b4342.
.
[TAR content here] 
##
T 127.0.0.1:8000 -> 127.0.0.1:36229 [AP]
HTTP/1.1 200 OK.
Transfer-Encoding: chunked.
X-Trans-Id: tx00000000000000000000b-0058f08913-1023-default.
Content-Type: text/plain; charset=utf-8.
Date: Fri, 14 Apr 2017 08:32:19 GMT.
Connection: close.
.

##
T 127.0.0.1:8000 -> 127.0.0.1:36229 [AP]
4d.
Number Files Created: 1
Response Body: 
Response Status: 201 Created
Errors: .
0.
.
@Jing-Scott

This comment has been minimized.

Contributor

Jing-Scott commented Apr 16, 2017

Ah... I see. I've tested them at the account. It works well. Thanks again!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment