diff --git a/CHANGELOG.md b/CHANGELOG.md index 4e727bb7917e87b42e67decb084da0c3c8aa2f28..54051db0d20ac2512bf95fcb96547892906304f0 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,6 +7,7 @@ Please mark all change in change log and use the issue from GitHub ## Bug - \#1762 Server is not forbidden to create new partition which tag is `_default` - \#1873 Fix index file serialize to incorrect path +- \#1881 Fix Annoy index search fail ## Feature - \#261 Integrate ANNOY into Milvus diff --git a/core/src/codecs/default/DefaultVectorIndexFormat.cpp b/core/src/codecs/default/DefaultVectorIndexFormat.cpp index 4ce2b2f4cd2976a463694f036f4107c0f978efd6..71df5ea73a16df89a02f51f2dd724ec729850ba9 100644 --- a/core/src/codecs/default/DefaultVectorIndexFormat.cpp +++ b/core/src/codecs/default/DefaultVectorIndexFormat.cpp @@ -36,9 +36,12 @@ DefaultVectorIndexFormat::read_internal(const storage::FSHandlerPtr& fs_ptr, con knowhere::BinarySet load_data_list; recorder.RecordSection("Start"); - fs_ptr->reader_ptr_->open(path); + if (!fs_ptr->reader_ptr_->open(path)) { + ENGINE_LOG_ERROR << "Fail to open vector index: " << path; + return nullptr; + } - size_t length = fs_ptr->reader_ptr_->length(); + int64_t length = fs_ptr->reader_ptr_->length(); if (length <= 0) { ENGINE_LOG_ERROR << "Invalid vector index length: " << path; return nullptr; @@ -128,7 +131,10 @@ DefaultVectorIndexFormat::write(const storage::FSHandlerPtr& fs_ptr, const std:: int32_t index_type = knowhere::StrToOldIndexType(index->index_type()); recorder.RecordSection("Start"); - fs_ptr->writer_ptr_->open(location); + if (!fs_ptr->writer_ptr_->open(location)) { + ENGINE_LOG_ERROR << "Fail to open vector index: " << location; + return; + } fs_ptr->writer_ptr_->write(&index_type, sizeof(index_type)); diff --git a/core/src/storage/IOReader.h b/core/src/storage/IOReader.h index eeddb7438bcfe97425526944c3d0b6ac3c63d7d8..98fabcefc4a040cafe86081f01d5da09bf989036 100644 --- a/core/src/storage/IOReader.h +++ b/core/src/storage/IOReader.h @@ -19,16 +19,16 @@ namespace storage { class IOReader { public: - virtual void + virtual bool open(const std::string& name) = 0; virtual void - read(void* ptr, size_t size) = 0; + read(void* ptr, int64_t size) = 0; virtual void - seekg(size_t pos) = 0; + seekg(int64_t pos) = 0; - virtual size_t + virtual int64_t length() = 0; virtual void diff --git a/core/src/storage/IOWriter.h b/core/src/storage/IOWriter.h index 38b688705720c78c7ad33e697117204306f74c14..763174b11c4f772a39e05950aa5371540d268483 100644 --- a/core/src/storage/IOWriter.h +++ b/core/src/storage/IOWriter.h @@ -19,13 +19,13 @@ namespace storage { class IOWriter { public: - virtual void + virtual bool open(const std::string& name) = 0; virtual void - write(void* ptr, size_t size) = 0; + write(void* ptr, int64_t size) = 0; - virtual size_t + virtual int64_t length() = 0; virtual void diff --git a/core/src/storage/disk/DiskIOReader.cpp b/core/src/storage/disk/DiskIOReader.cpp index f51ffef0b3871180a3b725326f949e46747a66ce..ba5aa30dd33525020972f5d695d8e2df47c50269 100644 --- a/core/src/storage/disk/DiskIOReader.cpp +++ b/core/src/storage/disk/DiskIOReader.cpp @@ -14,26 +14,27 @@ namespace milvus { namespace storage { -void +bool DiskIOReader::open(const std::string& name) { name_ = name; fs_ = std::fstream(name_, std::ios::in | std::ios::binary); + return fs_.good(); } void -DiskIOReader::read(void* ptr, size_t size) { +DiskIOReader::read(void* ptr, int64_t size) { fs_.read(reinterpret_cast(ptr), size); } void -DiskIOReader::seekg(size_t pos) { +DiskIOReader::seekg(int64_t pos) { fs_.seekg(pos); } -size_t +int64_t DiskIOReader::length() { fs_.seekg(0, fs_.end); - size_t len = fs_.tellg(); + int64_t len = fs_.tellg(); fs_.seekg(0, fs_.beg); return len; } diff --git a/core/src/storage/disk/DiskIOReader.h b/core/src/storage/disk/DiskIOReader.h index 09d2e27955929c81148f7f2d5e6d975e4f6f6196..363564c5a9a9608a3c9e6b62e9e30d76ac3c49ee 100644 --- a/core/src/storage/disk/DiskIOReader.h +++ b/core/src/storage/disk/DiskIOReader.h @@ -33,16 +33,16 @@ class DiskIOReader : public IOReader { DiskIOReader& operator=(DiskIOReader&&) = delete; - void + bool open(const std::string& name) override; void - read(void* ptr, size_t size) override; + read(void* ptr, int64_t size) override; void - seekg(size_t pos) override; + seekg(int64_t pos) override; - size_t + int64_t length() override; void diff --git a/core/src/storage/disk/DiskIOWriter.cpp b/core/src/storage/disk/DiskIOWriter.cpp index 63899463b1b76c048d0025713b156399d27058d8..75bc84e82ea6e77956553bf13e5c5582f7175d5c 100644 --- a/core/src/storage/disk/DiskIOWriter.cpp +++ b/core/src/storage/disk/DiskIOWriter.cpp @@ -14,20 +14,21 @@ namespace milvus { namespace storage { -void +bool DiskIOWriter::open(const std::string& name) { name_ = name; len_ = 0; fs_ = std::fstream(name_, std::ios::out | std::ios::binary); + return fs_.good(); } void -DiskIOWriter::write(void* ptr, size_t size) { +DiskIOWriter::write(void* ptr, int64_t size) { fs_.write(reinterpret_cast(ptr), size); len_ += size; } -size_t +int64_t DiskIOWriter::length() { return len_; } diff --git a/core/src/storage/disk/DiskIOWriter.h b/core/src/storage/disk/DiskIOWriter.h index d803c9dd397c3a221e34d97b2a2de77eccbf39be..051c9fb5fe44e146b69ff47cb8fd1902e416a6f1 100644 --- a/core/src/storage/disk/DiskIOWriter.h +++ b/core/src/storage/disk/DiskIOWriter.h @@ -33,13 +33,13 @@ class DiskIOWriter : public IOWriter { DiskIOWriter& operator=(DiskIOWriter&&) = delete; - void + bool open(const std::string& name) override; void - write(void* ptr, size_t size) override; + write(void* ptr, int64_t size) override; - size_t + int64_t length() override; void @@ -47,7 +47,7 @@ class DiskIOWriter : public IOWriter { public: std::string name_; - size_t len_; + int64_t len_; std::fstream fs_; }; diff --git a/core/src/storage/s3/S3IOReader.cpp b/core/src/storage/s3/S3IOReader.cpp index e8d073029a2a4836bccf8c9beb7f336d0d7aa3a9..a2b33ef3f906c4db43c200b733c008b35803cf66 100644 --- a/core/src/storage/s3/S3IOReader.cpp +++ b/core/src/storage/s3/S3IOReader.cpp @@ -15,24 +15,24 @@ namespace milvus { namespace storage { -void +bool S3IOReader::open(const std::string& name) { name_ = name; pos_ = 0; - S3ClientWrapper::GetInstance().GetObjectStr(name_, buffer_); + return (S3ClientWrapper::GetInstance().GetObjectStr(name_, buffer_).ok()); } void -S3IOReader::read(void* ptr, size_t size) { +S3IOReader::read(void* ptr, int64_t size) { memcpy(ptr, buffer_.data() + pos_, size); } void -S3IOReader::seekg(size_t pos) { +S3IOReader::seekg(int64_t pos) { pos_ = pos; } -size_t +int64_t S3IOReader::length() { return buffer_.length(); } diff --git a/core/src/storage/s3/S3IOReader.h b/core/src/storage/s3/S3IOReader.h index 3311ff129762e768d69e8cc54cafb3cbca858ae7..623a3ff878b3d87eac6f23e651aa4612bc61187d 100644 --- a/core/src/storage/s3/S3IOReader.h +++ b/core/src/storage/s3/S3IOReader.h @@ -32,16 +32,16 @@ class S3IOReader : public IOReader { S3IOReader& operator=(S3IOReader&&) = delete; - void + bool open(const std::string& name) override; void - read(void* ptr, size_t size) override; + read(void* ptr, int64_t size) override; void - seekg(size_t pos) override; + seekg(int64_t pos) override; - size_t + int64_t length() override; void @@ -50,7 +50,7 @@ class S3IOReader : public IOReader { public: std::string name_; std::string buffer_; - size_t pos_; + int64_t pos_; }; using S3IOReaderPtr = std::shared_ptr; diff --git a/core/src/storage/s3/S3IOWriter.cpp b/core/src/storage/s3/S3IOWriter.cpp index 9d00db3c83232a7dd2d40c5155699a43a9f9953b..68acb374a3fae41373359d140accb50888a709fd 100644 --- a/core/src/storage/s3/S3IOWriter.cpp +++ b/core/src/storage/s3/S3IOWriter.cpp @@ -15,20 +15,21 @@ namespace milvus { namespace storage { -void +bool S3IOWriter::open(const std::string& name) { name_ = name; len_ = 0; buffer_ = ""; + return true; } void -S3IOWriter::write(void* ptr, size_t size) { +S3IOWriter::write(void* ptr, int64_t size) { buffer_ += std::string(reinterpret_cast(ptr), size); len_ += size; } -size_t +int64_t S3IOWriter::length() { return len_; } diff --git a/core/src/storage/s3/S3IOWriter.h b/core/src/storage/s3/S3IOWriter.h index 712e74b722df0e48551a5684c3bed01ae5252ba9..ba1b3510ae6e70706da9cca428b1f7b6b7e3ae6f 100644 --- a/core/src/storage/s3/S3IOWriter.h +++ b/core/src/storage/s3/S3IOWriter.h @@ -32,13 +32,13 @@ class S3IOWriter : public IOWriter { S3IOWriter& operator=(S3IOWriter&&) = delete; - void + bool open(const std::string& name) override; void - write(void* ptr, size_t size) override; + write(void* ptr, int64_t size) override; - size_t + int64_t length() override; void @@ -46,7 +46,7 @@ class S3IOWriter : public IOWriter { public: std::string name_; - size_t len_; + int64_t len_; std::string buffer_; }; diff --git a/core/unittest/CMakeLists.txt b/core/unittest/CMakeLists.txt index 85855a11df6b98f7763520c85426f895eee3bf18..0c57df086a9c3c02ff48783c2c1b4768bcabeab1 100644 --- a/core/unittest/CMakeLists.txt +++ b/core/unittest/CMakeLists.txt @@ -208,4 +208,4 @@ add_subdirectory(metrics) add_subdirectory(scheduler) add_subdirectory(server) add_subdirectory(thirdparty) -#add_subdirectory(storage) +add_subdirectory(storage) diff --git a/core/unittest/storage/CMakeLists.txt b/core/unittest/storage/CMakeLists.txt index 75df76f197ac51cd8c433b616a7c805d4b0d9e8c..5294935af06abfb7bc00a580dade034567ea6661 100644 --- a/core/unittest/storage/CMakeLists.txt +++ b/core/unittest/storage/CMakeLists.txt @@ -12,7 +12,8 @@ #------------------------------------------------------------------------------- set(test_files - ${CMAKE_CURRENT_SOURCE_DIR}/test_s3_client.cpp +# ${CMAKE_CURRENT_SOURCE_DIR}/test_s3_client.cpp + ${CMAKE_CURRENT_SOURCE_DIR}/test_disk.cpp ${CMAKE_CURRENT_SOURCE_DIR}/utils.cpp ) diff --git a/core/unittest/storage/test_disk.cpp b/core/unittest/storage/test_disk.cpp new file mode 100644 index 0000000000000000000000000000000000000000..ef43594687a2e3d83075862819697d11c48b6c64 --- /dev/null +++ b/core/unittest/storage/test_disk.cpp @@ -0,0 +1,62 @@ +// Copyright (C) 2019-2020 Zilliz. All rights reserved. +// +// Licensed under the Apache License, Version 2.0 (the "License"); you may not use this file except in compliance +// with the License. You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software distributed under the License +// is distributed on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express +// or implied. See the License for the specific language governing permissions and limitations under the License. + +#include + +#include "easyloggingpp/easylogging++.h" +#include "storage/disk/DiskIOReader.h" +#include "storage/disk/DiskIOWriter.h" +#include "storage/utils.h" + +INITIALIZE_EASYLOGGINGPP + +TEST_F(StorageTest, DISK_RW_TEST) { + const std::string index_name = "/tmp/test_index"; + const std::string content = "abcdefg"; + + { + milvus::storage::DiskIOWriter writer; + ASSERT_TRUE(writer.open(index_name)); + size_t len = content.length(); + writer.write(&len, sizeof(len)); + writer.write((void*)(content.data()), len); + ASSERT_TRUE(len + sizeof(len) == writer.length()); + writer.close(); + } + + { + milvus::storage::DiskIOReader reader; + ASSERT_FALSE(reader.open("/tmp/notexist")); + ASSERT_TRUE(reader.open(index_name)); + int64_t length = reader.length(); + int64_t rp = 0; + reader.seekg(rp); + std::string content_out; + while (rp < length) { + size_t len; + reader.read(&len, sizeof(len)); + rp += sizeof(len); + reader.seekg(rp); + + auto data = new char[len]; + reader.read(data, len); + rp += len; + reader.seekg(rp); + + content_out += std::string(data, len); + + delete[] data; + } + + ASSERT_TRUE(content == content_out); + reader.close(); + } +}