Support remote file system for blob direct write file

[xingbowang]

Summary

This PR adds remote file system support for blob direct write files.

Remote file systems (e.g. distributed storage) may report a stale path-level file size for an actively-written blob file while the opened read handle can see the latest bytes. This mismatch caused reads to fail with a "Malformed blob file" corruption error.

Changes

New: blob_file_open_options.h
Introduces a file-open contract via FileOptions::io_options.property_bag:

• SetBlobFileActiveDirectWriteOpenMode() — marks file options to signal this is an active direct-write blob file
• IsBlobFileActiveDirectWriteOpenMode() — lets a FileSystem implementation detect the hint and adjust behavior (e.g., use an open-handle view for size queries)

blob_file_reader.cc

• After opening the file, calls FSRandomAccessFile::GetFileSize() on the open handle in addition to the path-level fs->GetFileSize()
• Prefers the larger of the two sizes — the open handle may expose more bytes on remote FS where path-level metadata is cached/stale
• Moves the footer size validation after file open so both size sources are available

blob_file_cache.cc

• Passes open_file_options with the active-direct-write hint set when allow_footer_skip_retry is true (i.e., the file is still being written)

blob_file_partition_manager.cc

• Sets the active-direct-write hint on the FileOptions used to open new blob files for writing

Tests

• BlobFileReaderTest.CreateReaderUsesOpenedFileSizeWhenPathSizeIsStale — unit test with a StalePathSizeFileSystem wrapper that returns size 0 at the path level but the real file is readable via the open handle
• DBBlobDirectWriteTest.DirectWriteUsesActiveFileOpenHintForRemoteFile — integration test with a RemoteBlobVisibilityFileSystem that verifies the active-hint flag is set on both writer and reader open calls, and that reads succeed during active direct writes

[github-actions]

⚠️ clang-tidy: 3 warning(s) on changed lines

Completed in 225.4s.

Summary by check

Check	Count
cppcoreguidelines-special-member-functions	1
performance-no-automatic-move	1
performance-unnecessary-copy-initialization	1
Total	3

Details

db/blob/blob_file_reader.cc (1 warning(s))

db/blob/blob_file_reader.cc:143:12: warning: constness of 'open_file_size_status' prevents automatic move [performance-no-automatic-move]

db/blob/db_blob_direct_write_test.cc (2 warning(s))

db/blob/db_blob_direct_write_test.cc:121:7: warning: class 'ActiveBlobVisibilityWritableFile' defines a non-default destructor but does not define a move constructor or a move assignment operator [cppcoreguidelines-special-member-functions]
db/blob/db_blob_direct_write_test.cc:208:23: warning: local copy 'tracked_path' of the variable 'fname' is never modified; consider avoiding the copy [performance-unnecessary-copy-initialization]

[github-actions]

✅ Claude Code Review

Auto-triggered after CI passed — reviewing commit de72196

---

Code Review: Support remote file system for blob direct write file

PR: Support remote file system for blob direct write file
Author: xingbowang
Files Changed: 6 files, +563 / -28 lines

---

Summary

This PR adds remote filesystem support for blob direct write files by preferring FSRandomAccessFile::GetFileSize() on the opened handle over path-level fs->GetFileSize() in BlobFileReader::OpenFile(), with fallback on NotSupported. It introduces a property_bag-based hint system so FileSystem implementations can distinguish active direct-write files. The approach follows the established pattern in table/format.cc:502-511.

---

Findings

HIGH: Inconsistency with table/format.cc fallback behavior

File: db/blob/blob_file_reader.cc

table/format.cc:504 falls back to path-level on any failure from handle GetFileSize. This PR falls back only on NotSupported and returns errors otherwise. This means transient errors (e.g., network issues) from the handle fail immediately rather than trying the path fallback. This may be intentional but should be documented since it deviates from the existing pattern.

Recommendation: Align with table/format.cc or add a comment explaining the stricter policy.

---

MEDIUM: allow_footer_skip_retry used as proxy for "active direct write" hint

File: db/blob/blob_file_cache.cc:78-88, 131-142

The active-direct-write hint is set whenever allow_footer_skip_retry=true. Today this coupling is correct (only ResolveBlobDirectWriteIndex passes true), but it's a semantic overload. A future caller using allow_footer_skip_retry for a different purpose would incorrectly set the hint.

Recommendation: Add a comment explaining the coupling.

---

MEDIUM: Duplicated FileOptions copy + hint-setting logic

File: db/blob/blob_file_cache.cc

The same 6-line FileOptions copy + hint pattern appears in both GetBlobFileReader and OpenBlobFileReaderUncached.

Recommendation: Extract a helper method.

---

LOW: Sync point removal from IO error test is correct

The "BlobFileReader::OpenFile:GetFileSize" sync point removal from BlobFileReaderIOErrorTest is correct — the primary path no longer calls fs->GetFileSize(), and error propagation is tested by the new CreateReaderPropagatesOpenedFileSizeError test.

---

SUGGESTION: Test coverage gaps

Tests cover the three main branches well. Missing: compressed blobs, skip_footer_validation=true path, and MultiGetBlob through the new size logic.

SUGGESTION: Test helper deduplication

The three new unit tests share substantial boilerplate that could be extracted into a common helper.

---

Positive Observations

1. Follows established table/format.cc handle-then-path pattern
2. Correct RAII cleanup on all error paths
3. Fully backward compatible (default NotSupported return triggers fallback)
4. Clean 3-way error handling (OK/NotSupported/error)
5. Well-structured test wrappers and integration test with RemoteBlobVisibilityFileSystem

Verdict

Approve with minor suggestions. Main actionable items:

1. Document or align the stricter fallback behavior vs table/format.cc (HIGH)
2. Comment the allow_footer_skip_retry / active-direct-write coupling (MEDIUM)
3. Extract duplicated FileOptions hint-setting code (MEDIUM)

---

ℹ️ About this response

Generated by Claude Code.
Review methodology: claude_md/code_review.md

Limitations:

• Claude may miss context from files not in the diff
• Large PRs may be truncated
• Always apply human judgment to AI suggestions

Commands:

• /claude-review [context] — Request a code review
• /claude-query <question> — Ask about the PR or codebase

[meta-codesync]

@xingbowang has imported this pull request. If you are a Meta employee, you can view this in D100002686.

[meta-codesync]

@xingbowang has imported this pull request. If you are a Meta employee, you can view this in D100002686.

[meta-codesync]

@xingbowang has imported this pull request. If you are a Meta employee, you can view this in D100002686.

[meta-codesync]

@xingbowang has imported this pull request. If you are a Meta employee, you can view this in D100002686.