Fix failures when opening arrays or groups with directory placeholder files. #5558

teo-tsirpanis · 2025-06-25T20:25:04Z

Applies #4102 to groups.

TYPE: BUG
DESC: Fixed opening groups on object storage, that contain directory placeholder files.

Also added a required C++ API.

bekadavis9

LGTM

tiledb/sm/group/group_directory.cc

rroelke · 2025-07-01T17:05:42Z

test/src/unit-cppapi-group.cc

@@ -75,7 +76,8 @@ struct GroupCPPFx {

 GroupCPPFx::GroupCPPFx()
    : ctx_c_(vfs_test_setup_.ctx_c)
-    , ctx_(vfs_test_setup_.ctx()) {
+    , ctx_(vfs_test_setup_.ctx())
+    , vfs_(ctx_, vfs_test_setup_.vfs_c, false) {


What's the granularity of this? Since you only need this for the new test (I presume) maybe this could be done in a subclass.

I could make the C++ VFS local to the test, but I don't see any problem with having the fixture class hold it.

test/src/unit-cppapi-group.cc

rroelke · 2025-07-01T17:23:59Z

tiledb/sm/group/group_directory.cc

+    }
+
+    // Filter out empty files of the same name as the directory
+    if (entry_uri.remove_trailing_slash() == uri.remove_trailing_slash() &&


uri.remove_trailing_slash() makes a copy of the underlying string. Not an expensive operation, but also one that we would like to avoid repeating for potentially every member of a directory. Best to move it out of the loop.

Though, this is not an ls_recursive, so in the typical case, we only expect to have __group, __meta, and __tiledb_group.tdb as directory entries, right?

uri.remove_trailing_slash() makes a copy of the underlying string. Not an expensive operation, but also one that we would like to avoid repeating for potentially every member of a directory. Best to move it out of the loop.

While this would be a good thing to do, it would probably be better still to avoid copying entirely by optimistically checking whether the paths match up to some reasonably-sized substring (perhaps all but a possible trailing slash), and only doing this remove_trailing_slash approach if so.

test/src/unit-cppapi-group.cc

rroelke · 2025-07-01T17:37:10Z

tiledb/sm/group/group_directory.cc

+std::vector<URI> ls(const VFS& vfs, const URI& uri) {
+  auto dir_entries = vfs.ls_with_sizes(uri);
+  auto& dirs = dir_names();
+  std::vector<URI> uris;


This should probably reserve something similar to dir_entries.size().
Is the case where we filter objects out a typical one? I'm supposing not since the previous implementation just returned everything?

rroelke · 2025-07-01T17:38:57Z

tiledb/sm/group/group_directory.cc

+    if (iter == dirs.end() || entry.file_size() > 0) {
+      uris.emplace_back(entry_uri);
+    } else {
+      // Handle MinIO-based s3 implementation limitation


This should be more detailed. It's not clear to me how the behavior difference noted in the issue comments would lead here

Added more comments, including link to minio issue.

rroelke · 2025-07-01T17:40:10Z

tiledb/sm/group/group_directory.cc

+    } else {
+      // Handle MinIO-based s3 implementation limitation
+      throw GroupDirectoryException(
+          "Cannot list given uri; File '" + entry_uri.to_string() +


What's the recovery? If the user removes the empty object, then that would fix it, right?
If the error says that then it the user probably has enough information to resolve this themselves, potentially avoiding a support case

Reworded message.

…ith-zero-length-objects-at

teo-tsirpanis · 2025-07-11T13:01:12Z

Validated again with the following program:

#include <tiledb>

int main() {
  Context ctx;

  Group g(
      ctx,
      "s3://tiledb-theodore/ephemeral/core266/"
      "072333E9-3D1E-4AFC-843B-B92D72CFB614",
      TILEDB_READ);

  std::cout << "Group metadata count: " << g.metadata_num() << std::endl;

  return 0;
}

teo-tsirpanis · 2025-07-11T16:43:36Z

/backport to release-2.28

github-actions · 2025-07-11T16:43:47Z

Started backporting to release-2.28: https://github.com/TileDB-Inc/TileDB/actions/runs/16225162743

…th directory placeholder files. (#5558) (#5583) Backport of #5558 to release-2.28 --- TYPE: BUG DESC: Fixed opening groups on object storage, that contain directory placeholder files. --------- Co-authored-by: Theodore Tsirpanis <[email protected]>

teo-tsirpanis changed the title ~~Teo/core 266 failure to open array from s3 with zero length objects at~~ Fixed failures when directory placeholder files exist in object storage. Jun 25, 2025

teo-tsirpanis force-pushed the teo/core-266-failure-to-open-array-from-s3-with-zero-length-objects-at branch from 50060c7 to bda1060 Compare June 25, 2025 21:40

teo-tsirpanis changed the title ~~Fixed failures when directory placeholder files exist in object storage.~~ Fix failures when directory placeholder files exist in object storage. Jun 25, 2025

teo-tsirpanis changed the title ~~Fix failures when directory placeholder files exist in object storage.~~ Fix failures when opening arrays or groups with directory placeholder files. Jun 25, 2025

teo-tsirpanis added 7 commits June 26, 2025 22:41

Update VFS::ls_with_sizes to exclude directory placeholder objects.

6eaa2f7

Update tiledb_vfs_ls to keep the existing behavior for compatibility.

b398cdd

Fix random IDE errors.

aa81757

Add test.

9c9e051

Also added a required C++ API.

Fix test failures.

cb7dd2d

Revert changes.

0a77c8b

Fix copying.

2bf83fb

teo-tsirpanis force-pushed the teo/core-266-failure-to-open-array-from-s3-with-zero-length-objects-at branch from 5da8d87 to 0029231 Compare June 26, 2025 20:02

Apply a targeted fix on GroupDirectory, mimicking #4102.

d39b664

teo-tsirpanis force-pushed the teo/core-266-failure-to-open-array-from-s3-with-zero-length-objects-at branch from 0029231 to d39b664 Compare June 26, 2025 20:05

CHECK->REQUIRE

a936292

teo-tsirpanis force-pushed the teo/core-266-failure-to-open-array-from-s3-with-zero-length-objects-at branch from 703acac to 2e28d37 Compare June 27, 2025 21:34

Fix test.

decb024

teo-tsirpanis force-pushed the teo/core-266-failure-to-open-array-from-s3-with-zero-length-objects-at branch from 2e28d37 to decb024 Compare June 27, 2025 22:29

teo-tsirpanis marked this pull request as ready for review June 28, 2025 19:19

teo-tsirpanis requested a review from ypatia June 28, 2025 19:19

bekadavis9 approved these changes Jul 1, 2025

View reviewed changes

ypatia reviewed Jul 1, 2025

View reviewed changes

tiledb/sm/group/group_directory.cc Outdated Show resolved Hide resolved

Use constants::group_dir_names.

f13f972

rroelke reviewed Jul 1, 2025

View reviewed changes

teo-tsirpanis added 4 commits July 8, 2025 23:14

Pre-size uris vector.

3d1266a

Address PR feedback.

03c1c3f

Merge branch 'main' into teo/core-266-failure-to-open-array-from-s3-w…

d70d48b

…ith-zero-length-objects-at

Correctly pre-size uris vector.

4bd19f1

teo-tsirpanis requested a review from ypatia July 10, 2025 14:51

Merge branch 'main' into teo/core-266-failure-to-open-array-from-s3-w…

5d97363

…ith-zero-length-objects-at

ypatia approved these changes Jul 11, 2025

View reviewed changes

ypatia added the backport release-2.28 label Jul 11, 2025

Merge branch 'main' into teo/core-266-failure-to-open-array-from-s3-w…

00fd59e

…ith-zero-length-objects-at

teo-tsirpanis merged commit 1288f9c into main Jul 11, 2025
56 checks passed

teo-tsirpanis deleted the teo/core-266-failure-to-open-array-from-s3-with-zero-length-objects-at branch July 11, 2025 16:43

github-actions bot mentioned this pull request Jul 11, 2025

[Backport release-2.28] Fix failures when opening arrays or groups with directory placeholder files. (#5558) #5583

Merged

Fix failures when opening arrays or groups with directory placeholder files. #5558

Fix failures when opening arrays or groups with directory placeholder files. #5558

Uh oh!

Conversation

teo-tsirpanis commented Jun 25, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

bekadavis9 left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

teo-tsirpanis commented Jul 11, 2025

Uh oh!

Uh oh!

teo-tsirpanis commented Jul 11, 2025

Uh oh!

github-actions bot commented Jul 11, 2025

Uh oh!

Uh oh!

teo-tsirpanis commented Jun 25, 2025 •

edited

Loading