From 2597bfcd245bfe9f6b74bce2e3a48bb7a4b89896 Mon Sep 17 00:00:00 2001 From: Zhang Yifan Date: Sat, 10 Oct 2020 14:00:17 +0800 Subject: [PATCH] fix: fix fds_service_test (#635) --- src/block_service/fds/fds_service.cpp | 41 +++++---------------- src/block_service/fds/fds_service.h | 5 --- src/block_service/test/fds_service_test.cpp | 26 ++++++------- 3 files changed, 21 insertions(+), 51 deletions(-) diff --git a/src/block_service/fds/fds_service.cpp b/src/block_service/fds/fds_service.cpp index 41f140c7f9..cf271c982c 100644 --- a/src/block_service/fds/fds_service.cpp +++ b/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 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(); }; diff --git a/src/block_service/fds/fds_service.h b/src/block_service/fds/fds_service.h index 3410336329..2d5ff9e8e3 100644 --- a/src/block_service/fds/fds_service.h +++ b/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; } diff --git a/src/block_service/test/fds_service_test.cpp b/src/block_service/test/fds_service_test.cpp index df1be1ab47..f85b313bbe 100644 --- a/src/block_service/test/fds_service_test.cpp +++ b/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 s = std::make_shared(); @@ -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 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());