Changes from review.

Author: shaunrd0

format_spec/FORMAT_SPEC.md

@@ -6,6 +6,8 @@ title: Format Specification
 
 * The current TileDB array format version number is **22** (`uint32_t`).
   * Other structures might be versioned separately.
+  * Readers should ignore files that do not follow the respective
+    format specification.
 * Data written by TileDB and referenced in this document is **little-endian**
   with the following exceptions:
 

format_spec/metadata.md

@@ -23,8 +23,6 @@ The metadata folder can contain any number of [timestamped](./timestamped_name.m
 
 ## 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

@@ -842,7 +842,7 @@ void ArrayDirectory::load_array_meta_uris() {
         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)) {
+      if (!unfiltered_uri.is_timestamped_name()) {
         resources_.get().logger()->debug(
             "Skipping partial array metadata file: '{}'", uri);
         continue;

tiledb/sm/filesystem/posix.cc

@@ -375,34 +375,35 @@ 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.
-    // If this penalty becomes noticeable, we should just duplicate
-    // this implementation in ls() and don't get the size
+
     if (next_path->d_type == DT_DIR) {
       entries.emplace_back(abspath, 0, true);
     } else {
-      uint64_t size = temp_metadata ? 0 : file_size(URI(abspath));
-      entries.emplace_back(abspath, size, false);
+      entries.emplace_back(abspath, file_size(URI(abspath)), false);
     }
   }
   return entries;
 }
 
 Status Posix::ls(
     const std::string& path, std::vector<std::string>* paths) const {
-  for (auto& fs : ls_with_sizes(URI(path))) {
-    paths->emplace_back(fs.path().native());
+  struct dirent* next_path = nullptr;
+  auto dir = PosixDIR::open(path);
+  if (dir.empty()) {
+    return {};
+  }
+
+  while ((next_path = readdir(dir.get())) != nullptr) {
+    if (!strcmp(next_path->d_name, ".") || !strcmp(next_path->d_name, "..")) {
+      continue;
+    }
+    std::string abspath = path + "/" + next_path->d_name;
+
+    if (next_path->d_type == DT_DIR) {
+      paths->emplace_back(abspath);
+    } else {
+      paths->emplace_back(abspath);
+    }
   }
 
   return Status::Ok();

tiledb/sm/filesystem/uri.cc

@@ -191,6 +191,49 @@ bool URI::is_tiledb() const {
   return utils::parse::starts_with(uri_, "tiledb://");
 }
 
+bool URI::is_timestamped_name() const {
+  std::string part = last_path_part();
+  // __1_2_<32-digit-UUID> must be at minimum 38 characters long.
+  if (!part.starts_with("__") || part.size() < 38) {
+    return false;
+  }
+
+  // Separator between t1_t2.
+  size_t t1_separator = part.find('_', 2);
+  // Separator between t2_uuid.
+  size_t t2_separator = part.find('_', t1_separator + 1);
+  if (t1_separator == std::string::npos || t2_separator == std::string::npos) {
+    return false;
+  }
+
+  // Validate the timestamps are formatted correctly.
+  std::string t1 = part.substr(2, t1_separator - 2);
+  std::string t2 = part.substr(t1_separator + 1, t2_separator - t1_separator - 1);
+  for (const auto& t : {t1, t2}) {
+    if (!std::ranges::all_of(t, [](const char c) { return std::isdigit(c); })) {
+      return false;
+    }
+  }
+
+  // Separator between uuid[_v].
+  size_t uuid_separator = part.find('_', t2_separator + 1);
+  std::string uuid = part.substr(t2_separator + 1, uuid_separator);
+  // UUIDs generated for timestamped names are 32 characters long.
+  if (uuid.size() != 32) {
+    return false;
+  }
+
+  // Version is optional and may not appear in files using a timestamped name.
+  if (uuid_separator != std::string::npos) {
+    std::string version = part.substr(uuid_separator + 1);
+    if (version.size() > std::to_string(constants::format_version).size()) {
+      return false;
+    }
+  }
+
+  return true;
+}
+
 std::optional<size_t> URI::get_storage_component_index(
     size_t start_index) const {
   // Find '://' between path and array name. iff it exists

tiledb/sm/filesystem/uri.h

@@ -226,6 +226,13 @@ class URI {
    */
   bool is_tiledb() const;
 
+  /**
+   * Checks if the last part of the URI is a valid timestamped name.
+   *
+   * @return True if the last part if a timestamped name, else false.
+   */
+  bool is_timestamped_name() const;
+
   /**
    * Get the position in the input URI where the embedded storage URI starts, if
    * one can be found.

tiledb/sm/filesystem/win.cc

@@ -384,24 +384,13 @@ std::vector<directory_entry> Win::ls_with_sizes(const URI& uri) const {
       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, temp_metadata ? 0 : size.QuadPart, false);
+        entries.emplace_back(file_path, size.QuadPart, false);
       }
     }
 

tiledb/sm/misc/constants.cc

@@ -261,9 +261,6 @@ 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,9 +260,6 @@ 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;