Skip partially written metadata files.

Author: shaunrd0

format_spec/metadata.md

@@ -17,12 +17,14 @@ my_array                            # array folder
 ```
 
 The metadata folder can contain any number of [timestamped](./timestamped_name.md):
-* [metadata files](#array-metadata-file)
+* [metadata files](#metadata-file)
 * [vacuum files](./vacuum_file.md)
 * **Note**: the timestamped names do _not_ include the format version.
 
 ## Metadata File
 
+Metadata files with the `.tmp` extension are used for local filesystems only, indicating the metadata is still being flushed to disk.
+
 The metadata file consists of a single [generic tile](./generic_tile.md), containing multiple entries with the following data:
 
 | **Field** | **Type** | **Description** |

tiledb/sm/array/array_directory.cc

@@ -366,19 +366,7 @@ Status ArrayDirectory::load() {
 
       // Load (in parallel) the array metadata URIs
       tasks.emplace_back(resources_.get().compute_tp().execute([&]() {
-        int delay = 250, max_retries = 25;
-        for (int retry = 0; retry <= max_retries; ++retry) {
-          if (load_array_meta_uris()) {
-            break;
-          }
-          // Retry is triggered if metadata files were still flushing to disk.
-          std::this_thread::sleep_for(std::chrono::milliseconds(delay));
-          resources_.get().logger()->info(
-              "Encountered partially written array metadata while opening the "
-              "array. Waiting {}ms and retrying.", delay);
-          // Apply (default REST) backoff and retry up to 25 times.
-          delay *= 1.25;
-        }
+        load_array_meta_uris();
         return Status::Ok();
       }));
     }
@@ -606,7 +594,7 @@ std::vector<URI> ArrayDirectory::ls(const URI& uri) const {
   std::vector<URI> uris;
   uris.reserve(dir_entries.size());
 
-  for (auto entry : dir_entries) {
+  for (const auto& entry : dir_entries) {
     auto entry_uri = URI(entry.path().native());
 
     // Always list directories
@@ -844,17 +832,20 @@ ArrayDirectory::load_consolidated_commit_uris(
   return {Status::Ok(), uris, uris_set};
 }
 
-bool ArrayDirectory::load_array_meta_uris() {
+void ArrayDirectory::load_array_meta_uris() {
   // Load the URIs in the array metadata directory
   std::vector<URI> array_meta_dir_uris;
   {
     auto timer_se = stats_->start_timer("list_array_meta_uris");
-    array_meta_dir_uris =
-        ls(uri_.join_path(constants::array_metadata_dir_name));
-  }
-  auto pred = [](const URI& uri) { return uri.to_string().ends_with(".tmp"); };
-  if (uri_.is_file() && std::ranges::any_of(array_meta_dir_uris, pred)) {
-    return false;
+    std::vector<URI> unfiltered_uris;
+    throw_if_not_ok(resources_.get().vfs().ls(
+        uri_.join_path(constants::array_metadata_dir_name), &unfiltered_uris));
+    for (const auto& unfiltered_uri : unfiltered_uris) {
+      const auto& uri = unfiltered_uri.to_string();
+      if (!uri.ends_with(constants::temp_file_suffix)) {
+        array_meta_dir_uris.emplace_back(uri);
+      }
+    }
   }
 
   // Compute array metadata URIs and vacuum URIs to vacuum. */
@@ -871,7 +862,6 @@ bool ArrayDirectory::load_array_meta_uris() {
   throw_if_not_ok(st2);
   array_meta_uris_ = std::move(array_meta_filtered_uris.value());
   array_meta_dir_uris.clear();
-  return true;
 }
 
 void ArrayDirectory::load_array_schema_uris() {

tiledb/sm/array/array_directory.h

@@ -726,7 +726,7 @@ class ArrayDirectory {
   load_consolidated_commit_uris(const std::vector<URI>& commits_dir_uris);
 
   /** Loads the array metadata URIs. */
-  bool load_array_meta_uris();
+  void load_array_meta_uris();
 
   /** Loads the array schema URIs. */
   void load_array_schema_uris();

tiledb/sm/filesystem/posix.cc

@@ -375,6 +375,15 @@ std::vector<directory_entry> Posix::ls_with_sizes(const URI& uri) const {
     if (!strcmp(next_path->d_name, ".") || !strcmp(next_path->d_name, ".."))
       continue;
     std::string abspath = path + "/" + next_path->d_name;
+    // Do not attempt to retrieve file size for temporary metadata files that
+    // are still flushing to disk.
+    const bool temp_metadata =
+        URI(abspath)
+            .parent_path()
+            .remove_trailing_slash()
+            .to_string()
+            .ends_with(constants::array_metadata_dir_name) &&
+        abspath.ends_with(constants::temp_file_suffix);
 
     // Getting the file size here incurs an additional system call
     // via file_size() and ls() calls will feel this too.
@@ -383,7 +392,7 @@ std::vector<directory_entry> Posix::ls_with_sizes(const URI& uri) const {
     if (next_path->d_type == DT_DIR) {
       entries.emplace_back(abspath, 0, true);
     } else {
-      uint64_t size = file_size(URI(abspath));
+      uint64_t size = temp_metadata ? 0 : file_size(URI(abspath));
       entries.emplace_back(abspath, size, false);
     }
   }

tiledb/sm/filesystem/win.cc

@@ -383,13 +383,25 @@ std::vector<directory_entry> Win::ls_with_sizes(const URI& uri) const {
         strcmp(find_data.cFileName, "..") != 0) {
       std::string file_path =
           path + (ends_with_slash ? "" : "\\") + find_data.cFileName;
+
+      // Do not attempt to retrieve file size for temporary metadata files that
+      // are still flushing to disk.
+      const bool temp_metadata =
+          URI(file_path)
+              .parent_path()
+              .remove_trailing_slash()
+              .to_string()
+              .ends_with(constants::array_metadata_dir_name) &&
+          file_path.ends_with(constants::temp_file_suffix);
+
       if (find_data.dwFileAttributes & FILE_ATTRIBUTE_DIRECTORY) {
         entries.emplace_back(file_path, 0, true);
       } else {
         ULARGE_INTEGER size;
         size.LowPart = find_data.nFileSizeLow;
         size.HighPart = find_data.nFileSizeHigh;
-        entries.emplace_back(file_path, size.QuadPart, false);
+        entries.emplace_back(
+            file_path, temp_metadata ? 0 : size.QuadPart, false);
       }
     }
 

tiledb/sm/metadata/metadata.cc

@@ -210,15 +210,14 @@ void Metadata::store(
   resources.stats().add_counter(
       "write_meta_size", size_computation_serializer.size());
 
-  // For LocalFS use a .tmp extension until write is flushed.
-  URI metadata_uri = get_uri(uri);
-  if (uri.is_file()) {
-    metadata_uri = URI(metadata_uri.to_string() + ".tmp");
-  }
-  // Create a metadata file name
-  GenericTileIO::store_data(resources, metadata_uri, tile, encryption_key);
   if (uri.is_file()) {
+    // For LocalFS use a .tmp extension until write is flushed.
+    URI metadata_uri(get_uri(uri).to_string() + ".tmp");
+    GenericTileIO::store_data(resources, metadata_uri, tile, encryption_key);
     resources.vfs().move_file(metadata_uri, get_uri(uri));
+  } else {
+    // Create a metadata file name
+    GenericTileIO::store_data(resources, get_uri(uri), tile, encryption_key);
   }
 }
 

tiledb/sm/misc/constants.cc

@@ -261,6 +261,9 @@ const std::string con_commits_file_suffix = ".con";
 /** Suffix for the special ignore files used in TileDB. */
 const std::string ignore_file_suffix = ".ign";
 
+/** Suffix for temporary array metadata files used in TileDB for LocalFS. */
+const std::string temp_file_suffix = ".tmp";
+
 /** Default datatype for a generic tile. */
 const Datatype generic_tile_datatype = Datatype::CHAR;
 

tiledb/sm/misc/constants.h

@@ -260,6 +260,9 @@ extern const std::string con_commits_file_suffix;
 /** Suffix for the special ignore files used in TileDB. */
 extern const std::string ignore_file_suffix;
 
+/** Suffix for temporary array metadata files used in TileDB for LocalFS. */
+extern const std::string temp_file_suffix;
+
 /** The fragment metadata file name. */
 extern const std::string fragment_metadata_filename;