Unverified Commit 89ca954e authored by 蔡宇东's avatar 蔡宇东 Committed by GitHub
Browse files

fix storage clang tidy warnings (#3285)



* update storage interface Read

Signed-off-by: default avataryudong.cai <yudong.cai@zilliz.com>

* enable storage unittest

Signed-off-by: default avataryudong.cai <yudong.cai@zilliz.com>

* use THROW_ERROR in more places

Signed-off-by: default avataryudong.cai <yudong.cai@zilliz.com>

* code opt

Signed-off-by: default avataryudong.cai <yudong.cai@zilliz.com>

* rename read() to Read()

Signed-off-by: default avataryudong.cai <yudong.cai@zilliz.com>

* update storage interface names

Signed-off-by: default avataryudong.cai <yudong.cai@zilliz.com>

* roll back to void Read()

Signed-off-by: default avataryudong.cai <yudong.cai@zilliz.com>
parent fae9ea33
Loading
Loading
Loading
Loading
+25 −42
Original line number Diff line number Diff line
@@ -32,52 +32,43 @@ namespace codec {

void
BlockFormat::Read(const storage::FSHandlerPtr& fs_ptr, const std::string& file_path, engine::BinaryDataPtr& raw) {
    if (!fs_ptr->reader_ptr_->open(file_path.c_str())) {
        std::string err_msg = "Failed to open file: " + file_path + ", error: " + std::strerror(errno);
        LOG_ENGINE_ERROR_ << err_msg;
        throw Exception(SERVER_CANNOT_OPEN_FILE, err_msg);
    if (!fs_ptr->reader_ptr_->Open(file_path)) {
        THROW_ERROR(SERVER_CANNOT_OPEN_FILE, "Fail to open file: " + file_path);
    }

    size_t num_bytes;
    fs_ptr->reader_ptr_->read(&num_bytes, sizeof(size_t));
    fs_ptr->reader_ptr_->Read(&num_bytes, sizeof(size_t));

    raw = std::make_shared<engine::BinaryData>();
    raw->data_.resize(num_bytes);
    fs_ptr->reader_ptr_->read(raw->data_.data(), num_bytes);

    fs_ptr->reader_ptr_->close();
    fs_ptr->reader_ptr_->Read(raw->data_.data(), num_bytes);
    fs_ptr->reader_ptr_->Close();
}

void
BlockFormat::Read(const storage::FSHandlerPtr& fs_ptr, const std::string& file_path, int64_t offset, int64_t num_bytes,
                  engine::BinaryDataPtr& raw) {
    if (offset < 0 || num_bytes <= 0) {
        std::string err_msg = "Invalid input to read: " + file_path;
        LOG_ENGINE_ERROR_ << err_msg;
        throw Exception(SERVER_INVALID_ARGUMENT, err_msg);
        THROW_ERROR(SERVER_INVALID_ARGUMENT, "Invalid input to read: " + file_path);
    }

    if (!fs_ptr->reader_ptr_->open(file_path.c_str())) {
        std::string err_msg = "Failed to open file: " + file_path + ", error: " + std::strerror(errno);
        LOG_ENGINE_ERROR_ << err_msg;
        throw Exception(SERVER_CANNOT_OPEN_FILE, err_msg);
    if (!fs_ptr->reader_ptr_->Open(file_path)) {
        THROW_ERROR(SERVER_CANNOT_OPEN_FILE, "Fail to open file: " + file_path);
    }

    size_t total_num_bytes;
    fs_ptr->reader_ptr_->read(&total_num_bytes, sizeof(size_t));
    fs_ptr->reader_ptr_->Read(&total_num_bytes, sizeof(size_t));

    offset += sizeof(size_t);  // Beginning of file is num_bytes
    if (offset + num_bytes > total_num_bytes) {
        std::string err_msg = "Invalid input to read: " + file_path;
        LOG_ENGINE_ERROR_ << err_msg;
        throw Exception(SERVER_INVALID_ARGUMENT, err_msg);
        THROW_ERROR(SERVER_INVALID_ARGUMENT, "Invalid argument to read: " + file_path);
    }

    raw = std::make_shared<engine::BinaryData>();
    raw->data_.resize(num_bytes);
    fs_ptr->reader_ptr_->seekg(offset);
    fs_ptr->reader_ptr_->read(raw->data_.data(), num_bytes);
    fs_ptr->reader_ptr_->close();
    fs_ptr->reader_ptr_->Seekg(offset);
    fs_ptr->reader_ptr_->Read(raw->data_.data(), num_bytes);
    fs_ptr->reader_ptr_->Close();
}

void
@@ -87,23 +78,18 @@ BlockFormat::Read(const storage::FSHandlerPtr& fs_ptr, const std::string& file_p
        return;
    }

    if (!fs_ptr->reader_ptr_->open(file_path.c_str())) {
        std::string err_msg = "Failed to open file: " + file_path + ", error: " + std::strerror(errno);
        LOG_ENGINE_ERROR_ << err_msg;
        throw Exception(SERVER_CANNOT_OPEN_FILE, err_msg);
    if (!fs_ptr->reader_ptr_->Open(file_path)) {
        THROW_ERROR(SERVER_CANNOT_OPEN_FILE, "Fail to open file: " + file_path);
    }

    size_t total_num_bytes;
    fs_ptr->reader_ptr_->read(&total_num_bytes, sizeof(size_t));
    fs_ptr->reader_ptr_->Read(&total_num_bytes, sizeof(size_t));

    int64_t total_bytes = 0;
    for (auto& range : read_ranges) {
        if (range.offset_ > total_num_bytes) {
            std::string err_msg = "Invalid input to read: " + file_path;
            LOG_ENGINE_ERROR_ << err_msg;
            throw Exception(SERVER_INVALID_ARGUMENT, err_msg);
            THROW_ERROR(SERVER_INVALID_ARGUMENT, "Invalid argument to read: " + file_path);
        }

        total_bytes += range.num_bytes_;
    }

@@ -112,12 +98,11 @@ BlockFormat::Read(const storage::FSHandlerPtr& fs_ptr, const std::string& file_p
    int64_t poz = 0;
    for (auto& range : read_ranges) {
        int64_t offset = range.offset_ + sizeof(size_t);
        fs_ptr->reader_ptr_->seekg(offset);
        fs_ptr->reader_ptr_->read(raw->data_.data() + poz, range.num_bytes_);
        fs_ptr->reader_ptr_->Seekg(offset);
        fs_ptr->reader_ptr_->Read(raw->data_.data() + poz, range.num_bytes_);
        poz += range.num_bytes_;
    }

    fs_ptr->reader_ptr_->close();
    fs_ptr->reader_ptr_->Close();
}

void
@@ -127,16 +112,14 @@ BlockFormat::Write(const storage::FSHandlerPtr& fs_ptr, const std::string& file_
        return;
    }

    if (!fs_ptr->writer_ptr_->open(file_path.c_str())) {
        std::string err_msg = "Failed to open file: " + file_path + ", error: " + std::strerror(errno);
        LOG_ENGINE_ERROR_ << err_msg;
        throw Exception(SERVER_CANNOT_CREATE_FILE, err_msg);
    if (!fs_ptr->writer_ptr_->Open(file_path)) {
        THROW_ERROR(SERVER_CANNOT_CREATE_FILE, "Fail to open file: " + file_path);
    }

    size_t num_bytes = raw->data_.size();
    fs_ptr->writer_ptr_->write(&num_bytes, sizeof(size_t));
    fs_ptr->writer_ptr_->write((void*)(raw->data_.data()), num_bytes);
    fs_ptr->writer_ptr_->close();
    fs_ptr->writer_ptr_->Write(&num_bytes, sizeof(size_t));
    fs_ptr->writer_ptr_->Write((void*)(raw->data_.data()), num_bytes);
    fs_ptr->writer_ptr_->Close();
}

}  // namespace codec
+19 −27
Original line number Diff line number Diff line
@@ -45,21 +45,19 @@ DeletedDocsFormat::Read(const storage::FSHandlerPtr& fs_ptr, const std::string&
                        segment::DeletedDocsPtr& deleted_docs) {
    const std::string full_file_path = file_path + DELETED_DOCS_POSTFIX;

    if (!fs_ptr->reader_ptr_->open(full_file_path)) {
        std::string err_msg = "Failed to open file: " + full_file_path;  // + ", error: " + std::strerror(errno);
        LOG_ENGINE_ERROR_ << err_msg;
        throw Exception(SERVER_CANNOT_CREATE_FILE, err_msg);
    if (!fs_ptr->reader_ptr_->Open(full_file_path)) {
        THROW_ERROR(SERVER_CANNOT_OPEN_FILE, "Fail to open deleted docs file: " + full_file_path);
    }

    size_t num_bytes;
    fs_ptr->reader_ptr_->read(&num_bytes, sizeof(size_t));
    fs_ptr->reader_ptr_->Read(&num_bytes, sizeof(size_t));

    auto deleted_docs_size = num_bytes / sizeof(engine::offset_t);
    std::vector<engine::offset_t> deleted_docs_list;
    deleted_docs_list.resize(deleted_docs_size);

    fs_ptr->reader_ptr_->read(deleted_docs_list.data(), num_bytes);
    fs_ptr->reader_ptr_->close();
    fs_ptr->reader_ptr_->Read(deleted_docs_list.data(), num_bytes);
    fs_ptr->reader_ptr_->Close();

    deleted_docs = std::make_shared<segment::DeletedDocs>(deleted_docs_list);
}
@@ -81,15 +79,13 @@ DeletedDocsFormat::Write(const storage::FSHandlerPtr& fs_ptr, const std::string&
    size_t old_num_bytes;
    std::vector<engine::offset_t> delete_ids;
    if (exists) {
        if (!fs_ptr->reader_ptr_->open(temp_path)) {
            std::string err_msg = "Failed to read from file: " + temp_path;  // + ", error: " + std::strerror(errno);
            LOG_ENGINE_ERROR_ << err_msg;
            throw Exception(SERVER_WRITE_ERROR, err_msg);
        if (!fs_ptr->reader_ptr_->Open(temp_path)) {
            THROW_ERROR(SERVER_CANNOT_OPEN_FILE, "Fail to open tmp deleted docs file: " + temp_path);
        }
        fs_ptr->reader_ptr_->read(&old_num_bytes, sizeof(size_t));
        fs_ptr->reader_ptr_->Read(&old_num_bytes, sizeof(size_t));
        delete_ids.resize(old_num_bytes / sizeof(engine::offset_t));
        fs_ptr->reader_ptr_->read(delete_ids.data(), old_num_bytes);
        fs_ptr->reader_ptr_->close();
        fs_ptr->reader_ptr_->Read(delete_ids.data(), old_num_bytes);
        fs_ptr->reader_ptr_->Close();
    } else {
        old_num_bytes = 0;
    }
@@ -100,15 +96,13 @@ DeletedDocsFormat::Write(const storage::FSHandlerPtr& fs_ptr, const std::string&
        delete_ids.insert(delete_ids.end(), deleted_docs_list.begin(), deleted_docs_list.end());
    }

    if (!fs_ptr->writer_ptr_->open(temp_path)) {
        std::string err_msg = "Failed to write from file: " + temp_path;  // + ", error: " + std::strerror(errno);
        LOG_ENGINE_ERROR_ << err_msg;
        throw Exception(SERVER_CANNOT_CREATE_FILE, err_msg);
    if (!fs_ptr->writer_ptr_->Open(temp_path)) {
        THROW_ERROR(SERVER_CANNOT_CREATE_FILE, "Fail to write file: " + temp_path);
    }

    fs_ptr->writer_ptr_->write(&new_num_bytes, sizeof(size_t));
    fs_ptr->writer_ptr_->write(delete_ids.data(), new_num_bytes);
    fs_ptr->writer_ptr_->close();
    fs_ptr->writer_ptr_->Write(&new_num_bytes, sizeof(size_t));
    fs_ptr->writer_ptr_->Write(delete_ids.data(), new_num_bytes);
    fs_ptr->writer_ptr_->Close();

    // Move temp file to delete file
    std::experimental::filesystem::rename(temp_path, full_file_path);
@@ -117,17 +111,15 @@ DeletedDocsFormat::Write(const storage::FSHandlerPtr& fs_ptr, const std::string&
void
DeletedDocsFormat::ReadSize(const storage::FSHandlerPtr& fs_ptr, const std::string& file_path, size_t& size) {
    const std::string full_file_path = file_path + DELETED_DOCS_POSTFIX;
    if (!fs_ptr->writer_ptr_->open(full_file_path)) {
        std::string err_msg = "Failed to open file: " + full_file_path;  // + ", error: " + std::strerror(errno);
        LOG_ENGINE_ERROR_ << err_msg;
        throw Exception(SERVER_CANNOT_CREATE_FILE, err_msg);
    if (!fs_ptr->writer_ptr_->Open(full_file_path)) {
        THROW_ERROR(SERVER_CANNOT_CREATE_FILE, "Fail to open deleted docs file: " + full_file_path);
    }

    size_t num_bytes;
    fs_ptr->reader_ptr_->read(&num_bytes, sizeof(size_t));
    fs_ptr->reader_ptr_->Read(&num_bytes, sizeof(size_t));

    size = num_bytes / sizeof(engine::offset_t);
    fs_ptr->reader_ptr_->close();
    fs_ptr->reader_ptr_->Close();
}

}  // namespace codec
+3 −9
Original line number Diff line number Diff line
@@ -46,9 +46,7 @@ IdBloomFilterFormat::Read(const storage::FSHandlerPtr& fs_ptr, const std::string
        new_scaling_bloom_from_file(BLOOM_FILTER_CAPACITY, BLOOM_FILTER_ERROR_RATE, full_file_path.c_str());
    fiu_do_on("bloom_filter_nullptr", bloom_filter = nullptr);
    if (bloom_filter == nullptr) {
        std::string err_msg = "Failed to read bloom filter from file: " + full_file_path + ". " + std::strerror(errno);
        LOG_ENGINE_ERROR_ << err_msg;
        throw Exception(SERVER_UNEXPECTED_ERROR, err_msg);
        THROW_ERROR(SERVER_UNEXPECTED_ERROR, "Fail to read bloom filter from file: " + full_file_path);
    }
    id_bloom_filter_ptr = std::make_shared<segment::IdBloomFilter>(bloom_filter);
}
@@ -58,9 +56,7 @@ IdBloomFilterFormat::Write(const storage::FSHandlerPtr& fs_ptr, const std::strin
                           const segment::IdBloomFilterPtr& id_bloom_filter_ptr) {
    const std::string full_file_path = file_path + BLOOM_FILTER_POSTFIX;
    if (scaling_bloom_flush(id_bloom_filter_ptr->GetBloomFilter()) == -1) {
        std::string err_msg = "Failed to write bloom filter to file: " + full_file_path + ". " + std::strerror(errno);
        LOG_ENGINE_ERROR_ << err_msg;
        throw Exception(SERVER_UNEXPECTED_ERROR, err_msg);
        THROW_ERROR(SERVER_UNEXPECTED_ERROR, "Fail to write bloom filter to file: " + full_file_path);
    }
}

@@ -71,9 +67,7 @@ IdBloomFilterFormat::Create(const storage::FSHandlerPtr& fs_ptr, const std::stri
    scaling_bloom_t* bloom_filter =
        new_scaling_bloom(BLOOM_FILTER_CAPACITY, BLOOM_FILTER_ERROR_RATE, full_file_path.c_str());
    if (bloom_filter == nullptr) {
        std::string err_msg = "Failed to read bloom filter from file: " + full_file_path + ". " + std::strerror(errno);
        LOG_ENGINE_ERROR_ << err_msg;
        throw Exception(SERVER_UNEXPECTED_ERROR, err_msg);
        THROW_ERROR(SERVER_UNEXPECTED_ERROR, "Failed to read bloom filter from file: " + full_file_path);
    }
    id_bloom_filter_ptr = std::make_shared<segment::IdBloomFilter>(bloom_filter);
}
+25 −28
Original line number Diff line number Diff line
@@ -85,51 +85,49 @@ StructuredIndexFormat::Read(const milvus::storage::FSHandlerPtr& fs_ptr, const s
    knowhere::BinarySet load_data_list;

    std::string full_file_path = file_path + STRUCTURED_INDEX_POSTFIX;
    if (!fs_ptr->reader_ptr_->open(full_file_path)) {
        LOG_ENGINE_ERROR_ << "Fail to open structured index: " << full_file_path;
        return;
    if (!fs_ptr->reader_ptr_->Open(full_file_path)) {
        THROW_ERROR(SERVER_CANNOT_OPEN_FILE, "Fail to open structured index: " + full_file_path);
    }
    int64_t length = fs_ptr->reader_ptr_->length();
    int64_t length = fs_ptr->reader_ptr_->Length();
    if (length <= 0) {
        LOG_ENGINE_ERROR_ << "Invalid structured index length: " << full_file_path;
        return;
        THROW_ERROR(SERVER_UNEXPECTED_ERROR, "Invalid structured index length: " + full_file_path);
    }

    size_t rp = 0;
    fs_ptr->reader_ptr_->seekg(0);
    fs_ptr->reader_ptr_->Seekg(0);

    int32_t data_type = 0;
    fs_ptr->reader_ptr_->read(&data_type, sizeof(data_type));
    fs_ptr->reader_ptr_->Read(&data_type, sizeof(data_type));
    rp += sizeof(data_type);
    fs_ptr->reader_ptr_->seekg(rp);
    fs_ptr->reader_ptr_->Seekg(rp);

    LOG_ENGINE_DEBUG_ << "Start to read_index(" << full_file_path << ") length: " << length << " bytes";
    while (rp < length) {
        size_t meta_length;
        fs_ptr->reader_ptr_->read(&meta_length, sizeof(meta_length));
        fs_ptr->reader_ptr_->Read(&meta_length, sizeof(meta_length));
        rp += sizeof(meta_length);
        fs_ptr->reader_ptr_->seekg(rp);
        fs_ptr->reader_ptr_->Seekg(rp);

        auto meta = new char[meta_length];
        fs_ptr->reader_ptr_->read(meta, meta_length);
        fs_ptr->reader_ptr_->Read(meta, meta_length);
        rp += meta_length;
        fs_ptr->reader_ptr_->seekg(rp);
        fs_ptr->reader_ptr_->Seekg(rp);

        size_t bin_length;
        fs_ptr->reader_ptr_->read(&bin_length, sizeof(bin_length));
        fs_ptr->reader_ptr_->Read(&bin_length, sizeof(bin_length));
        rp += sizeof(bin_length);
        fs_ptr->reader_ptr_->seekg(rp);
        fs_ptr->reader_ptr_->Seekg(rp);

        auto bin = new uint8_t[bin_length];
        fs_ptr->reader_ptr_->read(bin, bin_length);
        fs_ptr->reader_ptr_->Read(bin, bin_length);
        rp += bin_length;
        fs_ptr->reader_ptr_->seekg(rp);
        fs_ptr->reader_ptr_->Seekg(rp);

        std::shared_ptr<uint8_t[]> binptr(bin);
        load_data_list.Append(std::string(meta, meta_length), binptr, bin_length);
        delete[] meta;
    }
    fs_ptr->reader_ptr_->close();
    fs_ptr->reader_ptr_->Close();

    double span = recorder.RecordSection("End");
    double rate = length * 1000000.0 / span / 1024 / 1024;
@@ -148,28 +146,27 @@ StructuredIndexFormat::Write(const milvus::storage::FSHandlerPtr& fs_ptr, const
    std::string full_file_path = file_path + STRUCTURED_INDEX_POSTFIX;
    auto binaryset = index->Serialize(knowhere::Config());

    if (!fs_ptr->writer_ptr_->open(full_file_path)) {
        LOG_ENGINE_ERROR_ << "Fail to open structured index: " << full_file_path;
        return;
    if (!fs_ptr->writer_ptr_->Open(full_file_path)) {
        THROW_ERROR(SERVER_CANNOT_OPEN_FILE, "Fail to open structured index: " + full_file_path);
    }
    fs_ptr->writer_ptr_->write(&data_type, sizeof(data_type));
    fs_ptr->writer_ptr_->Write(&data_type, sizeof(data_type));

    for (auto& iter : binaryset.binary_map_) {
        auto meta = iter.first.c_str();
        size_t meta_length = iter.first.length();
        fs_ptr->writer_ptr_->write(&meta_length, sizeof(meta_length));
        fs_ptr->writer_ptr_->write((void*)meta, meta_length);
        fs_ptr->writer_ptr_->Write(&meta_length, sizeof(meta_length));
        fs_ptr->writer_ptr_->Write((void*)meta, meta_length);

        auto binary = iter.second;
        int64_t binary_length = binary->size;
        fs_ptr->writer_ptr_->write(&binary_length, sizeof(binary_length));
        fs_ptr->writer_ptr_->write((void*)binary->data.get(), binary_length);
        fs_ptr->writer_ptr_->Write(&binary_length, sizeof(binary_length));
        fs_ptr->writer_ptr_->Write((void*)binary->data.get(), binary_length);
    }

    fs_ptr->writer_ptr_->close();
    fs_ptr->writer_ptr_->Close();

    double span = recorder.RecordSection("End");
    double rate = fs_ptr->writer_ptr_->length() * 1000000.0 / span / 1024 / 1024;
    double rate = fs_ptr->writer_ptr_->Length() * 1000000.0 / span / 1024 / 1024;
    LOG_ENGINE_DEBUG_ << "StructuredIndexFormat::write(" << full_file_path << ") rate " << rate << "MB/s";
}

+11 −14
Original line number Diff line number Diff line
@@ -41,23 +41,21 @@ VectorCompressFormat::Read(const storage::FSHandlerPtr& fs_ptr, const std::strin
    milvus::TimeRecorder recorder("VectorCompressFormat::Read");

    const std::string full_file_path = file_path + VECTOR_COMPRESS_POSTFIX;
    if (!fs_ptr->reader_ptr_->open(full_file_path)) {
        LOG_ENGINE_ERROR_ << "Fail to open vector compress: " << full_file_path;
        return;
    if (!fs_ptr->reader_ptr_->Open(full_file_path)) {
        THROW_ERROR(SERVER_CANNOT_OPEN_FILE, "Fail to open vector compress file: " + full_file_path);
    }

    int64_t length = fs_ptr->reader_ptr_->length();
    int64_t length = fs_ptr->reader_ptr_->Length();
    if (length <= 0) {
        LOG_ENGINE_ERROR_ << "Invalid vector compress length: " << full_file_path;
        return;
        THROW_ERROR(SERVER_UNEXPECTED_ERROR, "Invalid vector compress length: " + full_file_path);
    }

    compress->data = std::shared_ptr<uint8_t[]>(new uint8_t[length]);
    compress->size = length;

    fs_ptr->reader_ptr_->seekg(0);
    fs_ptr->reader_ptr_->read(compress->data.get(), length);
    fs_ptr->reader_ptr_->close();
    fs_ptr->reader_ptr_->Seekg(0);
    fs_ptr->reader_ptr_->Read(compress->data.get(), length);
    fs_ptr->reader_ptr_->Close();

    double span = recorder.RecordSection("End");
    double rate = length * 1000000.0 / span / 1024 / 1024;
@@ -70,13 +68,12 @@ VectorCompressFormat::Write(const storage::FSHandlerPtr& fs_ptr, const std::stri
    milvus::TimeRecorder recorder("VectorCompressFormat::Write");

    const std::string full_file_path = file_path + VECTOR_COMPRESS_POSTFIX;
    if (!fs_ptr->writer_ptr_->open(full_file_path)) {
        LOG_ENGINE_ERROR_ << "Fail to open vector compress: " << full_file_path;
        return;
    if (!fs_ptr->writer_ptr_->Open(full_file_path)) {
        THROW_ERROR(SERVER_CANNOT_OPEN_FILE, "Fail to open vector compress: " + full_file_path);
    }

    fs_ptr->writer_ptr_->write(compress->data.get(), compress->size);
    fs_ptr->writer_ptr_->close();
    fs_ptr->writer_ptr_->Write(compress->data.get(), compress->size);
    fs_ptr->writer_ptr_->Close();

    double span = recorder.RecordSection("End");
    double rate = compress->size * 1000000.0 / span / 1024 / 1024;
Loading