Revert "address comments"

This reverts commit 61d05ff.
CalvinNeo · Aug 14, 2024 · a29b7cf · a29b7cf
1 parent ff13cae
commit a29b7cf
Show file tree

Hide file tree

Showing 4 changed files with 15 additions and 10 deletions.
diff --git a/dbms/src/Server/Server.cpp b/dbms/src/Server/Server.cpp
@@ -1439,7 +1439,6 @@ int Server::main(const std::vector<std::string> & /*args*/)
     if (minmax_index_cache_size)
         global_context->setMinMaxIndexCache(minmax_index_cache_size);
 
-    /// The vector index cache by number instead of bytes. Because it use `mmap` and let the operator system decide the memory usage.
     size_t vec_index_cache_entities = config().getUInt64("vec_index_cache_entities", 1000);
     if (vec_index_cache_entities)
         global_context->setVectorIndexCache(vec_index_cache_entities);

diff --git a/dbms/src/Storages/DeltaMerge/File/dtpb/dmfile.proto b/dbms/src/Storages/DeltaMerge/File/dtpb/dmfile.proto
@@ -62,9 +62,8 @@ message ColumnStat {
     optional uint64 array_sizes_bytes = 10;
     optional uint64 array_sizes_mark_bytes = 11;
 
-    reserved 101; // used before
     // TODO(vector-index) Support multiple vector index on the same column
-    optional VectorIndexFileProps vector_index = 102;
+    optional VectorIndexFileProps vector_index = 101;
 }
 
 message ColumnStats {

diff --git a/dbms/src/Storages/S3/FileCache.cpp b/dbms/src/Storages/S3/FileCache.cpp
@@ -288,10 +288,13 @@ FileSegmentPtr FileCache::getOrWait(const S3::S3FilenameView & s3_fname, const s
     table.set(s3_key, file_seg);
     lock.unlock();
 
-    ++PerfContext::file_cache.fg_download_from_s3;
-    fgDownload(s3_key, file_seg);
+    PerfContext::file_cache.fg_download_from_s3++;
+    fgDownload(lock, s3_key, file_seg);
     if (!file_seg || !file_seg->isReadyToRead())
-        throw Exception(ErrorCodes::S3_ERROR, "Download object {} failed", s3_key);
+        throw Exception( //
+            ErrorCodes::S3_ERROR,
+            "Download object {} failed",
+            s3_key);
 
     return file_seg;
 }
@@ -673,7 +676,7 @@ void FileCache::bgDownload(const String & s3_key, FileSegmentPtr & file_seg)
         [this, s3_key = s3_key, file_seg = file_seg]() mutable { download(s3_key, file_seg); });
 }
 
-void FileCache::fgDownload(const String & s3_key, FileSegmentPtr & file_seg)
+void FileCache::fgDownload(std::unique_lock<std::mutex> & cache_lock, const String & s3_key, FileSegmentPtr & file_seg)
 {
     SYNC_FOR("FileCache::fgDownload"); // simulate long s3 download
 
@@ -693,10 +696,14 @@ void FileCache::fgDownload(const String & s3_key, FileSegmentPtr & file_seg)
         file_seg->setStatus(FileSegment::Status::Failed);
         GET_METRIC(tiflash_storage_remote_cache, type_dtfile_download_failed).Increment();
         file_seg.reset();
-        remove(s3_key);
+        remove(cache_lock, s3_key);
     }
 
-    LOG_DEBUG(log, "foreground downloading => s3_key {} finished", s3_key);
+    LOG_DEBUG(
+        log,
+        "foreground downloading count {} => s3_key {} finished",
+        bg_downloading_count.load(std::memory_order_relaxed),
+        s3_key);
 }
 
 bool FileCache::isS3Filename(const String & fname)

diff --git a/dbms/src/Storages/S3/FileCache.h b/dbms/src/Storages/S3/FileCache.h
@@ -253,7 +253,7 @@ class FileCache
         const std::optional<UInt64> & filesize = std::nullopt);
 
     void bgDownload(const String & s3_key, FileSegmentPtr & file_seg);
-    void fgDownload(const String & s3_key, FileSegmentPtr & file_seg);
+    void fgDownload(std::unique_lock<std::mutex> & cache_lock, const String & s3_key, FileSegmentPtr & file_seg);
     void download(const String & s3_key, FileSegmentPtr & file_seg);
     void downloadImpl(const String & s3_key, FileSegmentPtr & file_seg);