fix: fix fds_service_test (#635)

src/block_service/fds/fds_service.cpp

@@ -398,20 +398,6 @@ fds_file_object::fds_file_object(fds_service *s,
 {
 }
 
-fds_file_object::fds_file_object(fds_service *s,
-                                 const std::string &name,
-                                 const std::string &fds_path,
-                                 const std::string &md5,
-                                 uint64_t size)
-    : block_file(name),
-      _service(s),
-      _fds_path(fds_path),
-      _md5sum(md5),
-      _size(size),
-      _has_meta_synced(true)
-{
-}
-
 fds_file_object::~fds_file_object() {}
 
 error_code fds_file_object::get_file_meta()
@@ -572,7 +558,7 @@ error_code fds_file_object::put_content(/*in-out*/ std::istream &is,
         return err;
     }
 
-    ddebug("start to check meta data after successfully wrote data to fds");
+    ddebug("start to synchronize meta data after successfully wrote data to fds");
     err = get_file_meta();
     if (err == ERR_OK) {
         transfered_bytes = _size;
@@ -653,14 +639,6 @@ dsn::task_ptr fds_file_object::read(const read_request &req,
 {
     read_future_ptr t(new read_future(code, cb, 0));
     t->set_tracker(tracker);
-    read_response resp;
-    if (_has_meta_synced && _md5sum.empty()) {
-        derror("fds read failed: meta not synced or md5sum empty when read (%s)",
-               _fds_path.c_str());
-        resp.err = dsn::ERR_OBJECT_NOT_FOUND;
-        t->enqueue_with(resp);
-        return t;
-    }
 
     add_ref();
     auto read_in_background = [this, req, t]() {
@@ -691,14 +669,6 @@ dsn::task_ptr fds_file_object::download(const download_request &req,
     download_future_ptr t(new download_future(code, cb, 0));
     t->set_tracker(tracker);
     download_response resp;
-    if (_has_meta_synced && _md5sum.empty()) {
-        derror("fds download failed: meta not synced or md5sum empty when download (%s)",
-               _fds_path.c_str());
-        resp.err = dsn::ERR_OBJECT_NOT_FOUND;
-        resp.downloaded_size = 0;
-        t->enqueue_with(resp);
-        return t;
-    }
 
     std::shared_ptr<std::ofstream> handle(new std::ofstream(
         req.output_local_name, std::ios::binary | std::ios::out | std::ios::trunc));
@@ -722,9 +692,16 @@ dsn::task_ptr fds_file_object::download(const download_request &req,
         resp.err =
             get_content_in_batches(req.remote_pos, req.remote_length, *handle, transfered_size);
         resp.downloaded_size = 0;
-        if (handle->tellp() != -1)
+        if (resp.err == ERR_OK && handle->tellp() != -1) {
             resp.downloaded_size = handle->tellp();
+        }
         handle->close();
+        if (resp.err != ERR_OK && dsn::utils::filesystem::file_exists(req.output_local_name)) {
+            derror_f("fail to download file {} from fds, remove localfile {}",
+                     _fds_path,
+                     req.output_local_name);
+            dsn::utils::filesystem::remove_path(req.output_local_name);
+        }
         t->enqueue_with(resp);
         release_ref();
     };

src/block_service/fds/fds_service.h

@@ -68,11 +68,6 @@ class fds_file_object : public block_file
 {
 public:
     fds_file_object(fds_service *s, const std::string &name, const std::string &fds_path);
-    fds_file_object(fds_service *s,
-                    const std::string &name,
-                    const std::string &fds_path,
-                    const std::string &md5,
-                    uint64_t size);
 
     virtual ~fds_file_object();
     virtual uint64_t get_size() override { return _size; }

src/block_service/test/fds_service_test.cpp

@@ -118,19 +118,18 @@ void FDSClientTest::TearDown() {}
 
 DEFINE_TASK_CODE(lpc_btest, TASK_PRIORITY_HIGH, dsn::THREAD_POOL_DEFAULT)
 
-// TODO(zhangyifan): the test could not pass, should fix.
 TEST_F(FDSClientTest, test_basic_operation)
 {
-    const char *files[] = {"/fdstest1/test1/test1",
-                           "/fdstest1/test1/test2",
-                           "/fdstest1/test2/test1",
-                           "/fdstest1/test2/test2",
-                           "/fdstest2/test2",
-                           "/fdstest3",
-                           "/fds_rootfile",
+    const char *files[] = {"/fdstest/fdstest1/test1/test1",
+                           "/fdstest/fdstest1/test1/test2",
+                           "/fdstest/fdstest1/test2/test1",
+                           "/fdstest/fdstest1/test2/test2",
+                           "/fdstest/fdstest2/test2",
+                           "/fdstest/fdstest3",
+                           "/fdstest/fds_rootfile",
                            nullptr};
     // ensure prefix_path is the prefix of some file in files
-    std::string prefix_path = std::string("/fdstest1/test1");
+    std::string prefix_path = std::string("/fdstest/fdstest1/test1");
     int total_files;
 
     std::shared_ptr<fds_service> s = std::make_shared<fds_service>();
@@ -264,12 +263,12 @@ TEST_F(FDSClientTest, test_basic_operation)
         std::cout << "test ls files" << std::endl;
 
         // list the root
-        std::cout << "list the root" << std::endl;
+        std::cout << "list the test root" << std::endl;
         std::vector<ls_entry> root = {
             {"fdstest1", true}, {"fdstest2", true}, {"fdstest3", false}, {"fds_rootfile", false}};
         std::sort(root.begin(), root.end(), entry_cmp);
 
-        s->list_dir(ls_request{"/"},
+        s->list_dir(ls_request{"/fdstest"},
                     lpc_btest,
                     [&l_resp](const ls_response &resp) { l_resp = resp; },
                     nullptr)
@@ -390,7 +389,7 @@ TEST_F(FDSClientTest, test_basic_operation)
     // try to read a non-exist file
     {
         std::cout << "test try to read non-exist file" << std::endl;
-        s->create_file(create_file_request{"fds_hellword", true},
+        s->create_file(create_file_request{"non_exist_file", true},
                        lpc_btest,
                        [&cf_resp](const create_file_response &r) { cf_resp = r; },
                        nullptr)
@@ -639,7 +638,6 @@ generate_file(const char *filename, unsigned long long file_size, char *block, u
     close(fd);
 }
 
-// TODO(zhangyifan): the test could not pass, should fix.
 TEST_F(FDSClientTest, test_concurrent_upload_download)
 {
     char block[1024];
@@ -756,7 +754,7 @@ TEST_F(FDSClientTest, test_concurrent_upload_download)
         for (unsigned int i = 0; i < total_files; ++i) {
             block_file_ptr p = block_files[i];
             dsn::task_ptr t =
-                p->download(download_request{filenames[i] + ".b"},
+                p->download(download_request{filenames[i] + ".b", 0, -1},
                             lpc_btest,
                             [&filenames, &filesize, &md5, i, p](const download_response &dr) {
                                 printf("file %s download finished\n", filenames[i].c_str());