[data_storage] implement nfs backend delete and exists

[wangxiyu191]

Summary

• Implement NFS backend file deletion using the URI path.
• Avoid deleting shared multi-block files except for blkid=0; missing or empty blkid is treated as block 0 for compatibility.
• Make Exist check the filesystem and keep MightExist cheap by returning true without filesystem IO.
• Extend NFS backend tests for delete, multi-block delete behavior, invalid blkid, missing/empty blkid, Exist, and MightExist.

Validation

• git diff --check
• bazel test --config=py312 --cache_test_results=no //kv_cache_manager/data_storage/test:NfsBackendTest --test_output=errors
• bazel build --config=py312 //kv_cache_manager:kv_cache_manager_bin

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 6c7c2a5d9b

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

• Open a pull request for review
• Mark a draft as ready
• Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

[qoderai]

👋 Review Summary

Solid implementation — the ShouldDeleteFile logic is clean, the from_chars parsing is correct and strict, and the overall pattern is consistent with the existing Hf3fsBackend. Good use of non-throwing std::error_code overloads throughout.

🛡️ Key Risks & Issues

No blocking issues found. The core logic is sound:

• ShouldDeleteFile correctly treats missing/empty blkid as block-0 (delete), rejects non-numeric values with EC_ERROR, and skips deletion for non-zero blocks.
• std::from_chars with uint64_t naturally rejects negative values and non-numeric input.
• The std::filesystem::remove call gracefully handles already-deleted files (returns false without error, logged as a warning).

One design note worth considering: The Hf3fsBackend::Delete (lines 98-121 in hf3fs_backend.cc) unconditionally deletes the file for every URI, with no blkid-awareness. If hf3fs is also used with multi-block files (key_count_per_file > 1), it would delete the shared file on every block's Delete call, which could race with reads of other blocks. If that's an intentional difference, a brief comment in hf3fs_backend.cc would help future maintainers. If not, the ShouldDeleteFile guard could be promoted to a shared utility.

🧪 Verification Advice

The tests cover the critical scenarios well (block-0, non-zero, invalid, missing, empty blkid, real filesystem Exist, MightExist always-true). A few minor gaps to consider:

• Mixed-URI batch Delete: All multi-block tests call Delete with a single URI at a time. A test calling Delete({blkid=0_uri, blkid=1_uri, invalid_uri}) in one batch would verify the loop correctly produces [EC_OK, EC_OK, EC_ERROR] in a single invocation.
• Null callback: The code guards with if (cb) before invoking, but all tests pass a non-null lambda. One test with a default-constructed std::function<void()>{} would confirm no crash.

These are minor — the current coverage is sufficient for the core behavioral contract.

💡 Thoughts & Suggestions

• Exist operates at file granularity, not block granularity — for multi-block URIs all blocks sharing the same file will report the same existence status. This is consistent with Hf3fsBackend but worth a brief inline comment for future readers.
• The MightExist override is a nice touch — keeping it as a cheap always-true avoids unnecessary filesystem I/O for the fast-path check, which aligns well with the base class contract.

---

🤖 Generated by Qoder • View workflow run

[oldsharp]

这里把error code叫成decision有点困惑