rgw:add a s3 API of make torrent for a object #9589

zhouruisong · 2016-06-08T09:00:40Z

when you put a object, a torrent data of the object will be produced that saved in omap.When you execute the command gettorrent of a object, the torrent data of the object will be return from omap. The torrent data also be removed when the object is removed.
Signed-off-by: zhouruisong 236131368@qq.com

oritwas · 2016-06-09T08:45:43Z

@zhouruisong , can you open a tracker issue for this new feature and it to the commit message
(Fixes: )
why do you think you need a new pool?

mattbenjamin · 2016-06-10T17:50:20Z

@zhouruisong I'd like to get feedback from various folks on the different aspects of this, such as the torrent pool concept (e.g., how that might relate to storage classes), and maybe whether this capability should have its own RGWOp.

dang · 2016-06-10T18:59:19Z

@zhouruisong So, I'm not liking a separate pool to store torrent files. How about, instead, the torrent file is generated each time, and the computationally intensive portions of it (such as the SHA) are stored on the primary object as attributes? That way, when the object is removed it's torrent is removed, and no extra pools or related files are required.

zhouruisong · 2016-06-12T01:59:34Z

@oritwas tracker.ceph.com/issues/16241

zhouruisong · 2016-06-12T02:07:05Z

@oritwas I need a new torrent pool because I want to save torrent files Individually. it is acceptable to save pool that has existed. I will modify it in the future.

zhouruisong · 2016-06-12T02:13:37Z

@dang It will cost some time to produce torrent file when you get a object. I test that it cost about two seconds to produce a torrent file. The torrent file's source objcet is about 1.5G. So, I save the torrent file into ceph. if it exists, we wiil get it quickly. I will remove the torrent pool and save torrent file in existed pool. thanks!

zhouruisong · 2016-06-12T02:43:40Z

@dang Now the torrent file name and torrent pool name are saved in attributes, torrent file is saved in torrent pool.Do you mean save torrent file in omap?

mattbenjamin · 2016-06-13T15:21:55Z

@zhouruisong I think @dang is suggesting a strategy to materialize checksums/hashes, but not the torrent data; if someone really wants this kind of space amplification, I don't see how to argue against it for such users, provided there is a durable perf benefit (but that's worth digging more at), but b) I don't think we should force the space amp on everyone

zhouruisong · 2016-06-14T01:43:45Z

@mattbenjamin The torrent data is only 64k for a object whose capacity is 1.5G. A object whose capacity is 100G only occupy a few megabytes. So I thinks it is will not occupy too mush space for a torrent data of a object. I see your worry. I hope you give me some suggestion for the solution. thanks！

cbodley · 2016-06-14T20:45:41Z

src/rgw/rgw_rest.cc

+    torrent.set_encoding(g_conf->torrent_encoding);
+    torrent.set_origin(g_conf->torrent_origin);
+    string name = obj.get_orig_obj();
+    torrent.set_info_name(name);


i think that using get_orig_obj() to generate the torrent object names will prevent us from supporting object versioning and object instances. if you overwrite the original object with new data, you'd still find and return the old torrent object for it

i'd lean towards using the omap for this storage so that it's tied to a specific object version/instance, and gets cleaned up automatically when the object is deleted

thanks, I see what you said and I will modify it

I need the object name of torrent data beacuse there is a field in torrent file format that is called name。 For example： There is a object named 1.MP4 and when you get its torrent data, a torrent file named 1.MP4.torrent will be produced.But in 1.MP4.torrent, there is a filed name to save the source file. I need get this name when put object and save it into torrent data when producing torrent data. I do not need get the name when get object. If I do not get the object name in geting object, where can I get it? thanks!

get_orig_obj() is fine for the torrent info name and filename of the .torrent. the problem was only in the object name used to store it in rados

zhouruisong · 2016-06-15T10:42:35Z

@dang I have updated my code according to your suggestion, please help me check thanks!

zhouruisong · 2016-06-15T10:42:54Z

@mattbenjamin I have updated my code according to your suggestion, please help me check thanks!

zhouruisong · 2016-06-15T10:43:21Z

@cbodley I have updated my code according to your suggestion, please help me check thanks!

zhouruisong · 2016-06-15T10:43:30Z

@liewegas I have updated my code according to your suggestion, please help me check thanks!

mattbenjamin · 2016-06-15T20:56:29Z

@zhouruisong lgtm, looking for feedback from @dang

zhouruisong · 2016-06-16T01:55:01Z

@dang I have modified my code according to your suggestion. Now, torrent data has been saved into omap,and it will be removed when the object is removed. please give me your advice, thanks!

cbodley · 2016-06-16T13:19:10Z

src/rgw/rgw_op.cc

+
+  return op_ret;
+}
+


this seed class belongs in a separate file, like rgw_torrent.cc/h

zhouruisong · 2016-06-17T13:41:13Z

@dang I have updated my code again according to your advice,please help me check, thanks!

zhouruisong · 2016-06-17T13:41:23Z

@cbodley I have updated my code again according to your advice,please help me check, thanks!

dang · 2016-06-17T14:04:37Z

LGTM

zhouruisong · 2016-07-04T03:37:46Z

@dang What is the progress of the feature? I have test it and not find any problem

When you execute the command gettorrent of a object, a torrent file will be produced and returned. The torrent also will be save into a pool named default.rgw.torrent. If the torrent of a object exists in default.rgw.torrent, it will be returned. Signed-off-by: zhouruisong <236131368@qq.com>

dang · 2016-07-06T14:04:18Z

@zhouruisong I'm waiting for unit tests to be added to the PR, as requested by @cbodley

zhouruisong · 2016-07-07T08:52:39Z

@dang I want to know how to write test code? can you give me a example? Do I have to change some interface of the class seed from private to public?

cbodley · 2016-07-07T13:28:37Z

@zhouruisong i see what you mean: all of the encoding logic is tightly coupled to your seed class. moving that stuff into a separate class (or maybe free functions?) would make it easier to write tests against. consider an interface like this, which is similar to Ceph's existing serialization framework:

// control characters
void bencode_dict(bufferlist& bl) { bl.append('d'); }
void bencode_list(bufferlist& bl) { bl.append('l'); }
void bencode_end(bufferlist& bl) { bl.append('e'); }

// single values
void bencode(int i, bufferlist& bl);
void bencode(const std::string& str, bufferlist& bl);

// dictionary elements
void bencode(const std::string& key, int value, bufferlist& bl);
void bencode(const std::string& key, const std::string& value, bufferlist& bl);

Then an example unit test could look like this:

TEST(Bencode, Dict)
{
  bufferlist bl;

  bencode_dict(bl);
    bencode("foo", 5, bl);
    bencode("bar", "baz", bl);
  bencode_end(bl);

  ASSERT_EQ("d3fooi5e3bar3baze", bl.c_str());
}

cbodley · 2016-07-07T13:37:14Z

p.s. you can find a simple example test in src/test/test_pageset.cc. copy that to src/test/rgw/test_bencode.cc to get started

zhouruisong · 2016-07-11T06:35:36Z

@cbodley Do I run make check to run my unit test? All test will be run, how to make it check my own unit test?

zhouruisong · 2016-07-11T09:47:42Z

@cbodley I have update my code and add some unit test codes, but when I run make check, I do not konw my unit test have been excuted, please look at my unit test code, I do not know whether the codes in CMakeLists.txt and Makefile-client.am have been executed. Where did I ignore？

cbodley · 2016-07-11T14:06:58Z

src/test/rgw/CMakeLists.txt

+  ${CRYPTO_LIBS}
+  )
+set_target_properties(ceph_test_rgw_bencode PROPERTIES COMPILE_FLAGS
+  ${UNITTEST_CXX_FLAGS})


this is all you should need here:

add_executable(unittest_rgw_bencode test_rgw_bencode.cc) add_ceph_unittest(unittest_rgw_bencode ${CMAKE_RUNTIME_OUTPUT_DIRECTORY}/unittest_rgw_bencode) target_link_libraries(unittest_rgw_bencode rgw_a)

the add_ceph_unittest() part is what adds it to make check

@cbodley I have updated my code. but the unittest_rgw_bencode can not generate when I run make check.
I modify add "#define ENABLE_CLIENT 1" in src/acconfig.h and run make check again. the unittest_rgw_bencode can not generate. I think there are something configrations that I have ignored. I want to know why unittest_rgw_bencode can not be produced? Is it wrong with my code? thanks!

zhouruisong · 2016-07-13T03:14:52Z

@dang I have finished unit test code.

cbodley · 2016-07-13T12:55:48Z

src/test/rgw/test_rgw_bencode.cc

@@ -13,7 +13,8 @@ TEST(Bencode, Str)
  decode.bencode("bar", bl);
  decode.bencode("baz", bl);

-  ASSERT_EQ("3:foo3:bar3:baz", bl.c_str());
+  const char *p = "3:foo3:bar3:baz";
+  ASSERT_EQ(0, strcmp(p, bl.c_str()));


oops, ASSERT_STREQ() is what you want for const char* comparison (see https://github.com/google/googletest/blob/master/googletest/docs/Primer.md)

@cbodley I have updated this code, thank you for your advice

zhouruisong · 2016-07-15T02:38:18Z

@cbodley thank you for your advice and I will update my code.

dang · 2016-07-15T14:09:12Z

This is looking good. @cbodley is out today, but hopefully he can do a final review Monday to make sure all his comments have been addressed.

After that, could you squash all the commits down into a single commit with a good commit message?

dang · 2016-07-21T16:06:15Z

@zhouruisong I had to revert this. It was causing crashes on normal put. It appears that this was not the latest version we reviewed. Did something get messed up in the squash?

liewegas added feature rgw labels Jun 8, 2016

mattbenjamin self-assigned this Jun 10, 2016

mattbenjamin added the pending-discussion label Jun 10, 2016

cbodley reviewed Jun 14, 2016
View reviewed changes

zhouruisong force-pushed the master branch from 5b573f3 to c956894 Compare June 15, 2016 10:12

cbodley reviewed Jun 16, 2016
View reviewed changes

src/rgw/rgw_op.cc

return op_ret;

}

Copy link

Contributor

cbodley Jun 16, 2016

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this seed class belongs in a separate file, like rgw_torrent.cc/h

zhouruisong force-pushed the master branch from c956894 to e5da9e7 Compare June 17, 2016 13:36

mattbenjamin assigned dang and unassigned mattbenjamin Jun 29, 2016

zhouruisong force-pushed the master branch 2 times, most recently from 6b534d2 to 53d2829 Compare July 6, 2016 09:21

cbodley reviewed Jul 11, 2016
View reviewed changes

cbodley removed the needs-test label Jul 12, 2016

cbodley reviewed Jul 13, 2016
View reviewed changes

zhouruisong force-pushed the master branch from b4e2c21 to 53d2829 Compare July 18, 2016 09:17

dang merged commit f2bea59 into ceph:master Jul 21, 2016

This was referenced Jul 22, 2016

rgw:add a s3 API of make torrent for a object #10395

Closed

rgw:add a s3 API of make torrent for a object #10396

Merged

rgw:add a s3 API of make torrent for a object #9589

rgw:add a s3 API of make torrent for a object #9589

Conversation

zhouruisong commented Jun 8, 2016 • edited

oritwas commented Jun 9, 2016

mattbenjamin commented Jun 10, 2016

dang commented Jun 10, 2016 • edited by mattbenjamin

zhouruisong commented Jun 12, 2016

zhouruisong commented Jun 12, 2016 • edited

zhouruisong commented Jun 12, 2016

zhouruisong commented Jun 12, 2016 • edited

mattbenjamin commented Jun 13, 2016

zhouruisong commented Jun 14, 2016

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

zhouruisong commented Jun 15, 2016

zhouruisong commented Jun 15, 2016

zhouruisong commented Jun 15, 2016

zhouruisong commented Jun 15, 2016

mattbenjamin commented Jun 15, 2016

zhouruisong commented Jun 16, 2016 • edited

Choose a reason for hiding this comment

zhouruisong commented Jun 17, 2016

zhouruisong commented Jun 17, 2016

dang commented Jun 17, 2016

zhouruisong commented Jul 4, 2016

dang commented Jul 6, 2016

zhouruisong commented Jul 7, 2016 • edited

cbodley commented Jul 7, 2016

cbodley commented Jul 7, 2016

zhouruisong commented Jul 11, 2016

zhouruisong commented Jul 11, 2016

Choose a reason for hiding this comment

zhouruisong Jul 12, 2016 • edited

Choose a reason for hiding this comment

zhouruisong commented Jul 13, 2016

Choose a reason for hiding this comment

Choose a reason for hiding this comment

zhouruisong commented Jul 15, 2016

dang commented Jul 15, 2016

dang commented Jul 21, 2016

zhouruisong commented Jun 8, 2016 •

edited

dang commented Jun 10, 2016 •

edited by mattbenjamin

zhouruisong commented Jun 12, 2016 •

edited

zhouruisong commented Jun 12, 2016 •

edited

zhouruisong commented Jun 16, 2016 •

edited

zhouruisong commented Jul 7, 2016 •

edited

zhouruisong Jul 12, 2016 •

edited