[PR #562] [MERGED] refactor: introduce expected pattern for error handling in master service #1653

New issue

Closed

opened 2026-10-29 21:29:13 +00:00 by bsakaguchi · 0 comments

bsakaguchi commented

2026-10-29 21:29:13 +00:00

Owner

📋 Pull Request Information

Original PR: https://github.com/kvcache-ai/Mooncake/pull/562
Author: @xiaguan
Created: 6/26/2025
Status: ✅ Merged
Merged: 7/3/2025
Merged by: @xiaguan

Base: main ← Head: master_expected

📝 Commits (10+)

8643435 refactor: introduce expected pattern for error handling in master service
c34f5c4 Refactor error handling to use expected<T,E> pattern instead of ErrorCode
0302716 fix test compile
a61e14d refactor: update client implementation and remove master.proto
a2005d2 fix: update Python integration to work with new batch API
a9f1a5d refine master log and metric
3d0196f fix(ci): should alloc first
4cd8851 Merge branch 'main' into master_expected
2ae4bb6 merge main
de2d387 fix tests

📊 Changes

25 files changed (+2814 additions, -2119 deletions)

View changed files

📝 .gitignore (+3 -0)
📝 mooncake-integration/store/store_py.cpp (+296 -187)
📝 mooncake-integration/store/store_py.h (+7 -2)
📝 mooncake-store/include/cachelib_memory_allocator/Slab.h (+1 -1)
📝 mooncake-store/include/client.h (+69 -60)
📝 mooncake-store/include/master_client.h (+53 -48)
📝 mooncake-store/include/master_metric_manager.h (+36 -0)
📝 mooncake-store/include/master_service.h (+61 -50)
➕ mooncake-store/include/rpc_helper.h (+60 -0)
📝 mooncake-store/include/rpc_service.h (+299 -366)
📝 mooncake-store/include/types.h (+4 -3)
📝 mooncake-store/include/utils.h (+73 -1)
📝 mooncake-store/include/utils/scoped_vlog_timer.h (+37 -6)
➖ mooncake-store/proto/master.proto (+0 -225)
📝 mooncake-store/src/client.cpp (+650 -366)
📝 mooncake-store/src/master_client.cpp (+229 -263)
📝 mooncake-store/src/master_metric_manager.cpp (+106 -0)
📝 mooncake-store/src/master_service.cpp (+151 -161)
📝 mooncake-store/tests/client_integration_test.cpp (+129 -75)
📝 mooncake-store/tests/e2e/client_wrapper.cpp (+21 -13)

...and 5 more files

📄 Description

Overview

This PR introduces the tl::expected pattern for error handling in the master service, replacing traditional ErrorCode return types with a more type-safe and explicit approach.

Changes

Core Improvements

Type-safe error handling: Replace ErrorCode return types with tl::expected<T, ErrorCode> pattern
Clearer API semantics: Success values and error codes are now explicitly separated
Better developer experience:
- Success cases: access via .value()
- Error cases: access via .error()
- Eliminates ambiguity between success and error states

Modified Components

MasterService methods: Updated to return expected<void, ErrorCode> or expected<T, ErrorCode>
RPC service interfaces: Modified to support the expected pattern
Test suites: Updated all related tests to handle new expected return types
Dependencies: Added necessary includes for ylt/util/expected.hpp

Files Changed

mooncake-store/include/master_service.h - Updated method signatures
mooncake-store/include/rpc_service.h - RPC interface adaptations
mooncake-store/include/types.h - Minor formatting improvements
mooncake-store/src/master_service.cpp - Implementation updates
mooncake-store/tests/ - Comprehensive test updates

Benefits

Type Safety: Compile-time guarantees about error handling
Explicit Error Handling: No more ambiguous return codes
Better Maintainability: Clear separation of success and error paths
Future-Ready: Foundation for extending expected pattern to RPC responses

Future Work

This PR lays the groundwork for:

Extending expected pattern to RPC response types
Enhancing the error code system for more comprehensive error handling
Further improving type safety across the codebase

Testing

All existing tests have been updated and continue to pass, ensuring backward compatibility and correctness of the new error handling approach.

Note: This change maintains API compatibility while improving error handling semantics. The expected pattern provides a modern, type-safe approach to error handling that will benefit future development and maintenance.

_{🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.}

## 📋 Pull Request Information **Original PR:** https://github.com/kvcache-ai/Mooncake/pull/562 **Author:** [@xiaguan](https://github.com/xiaguan) **Created:** 6/26/2025 **Status:** ✅ Merged **Merged:** 7/3/2025 **Merged by:** [@xiaguan](https://github.com/xiaguan) **Base:** `main` ← **Head:** `master_expected` --- ### 📝 Commits (10+) - [`8643435`](https://github.com/kvcache-ai/Mooncake/commit/864343521b81ee259c71a625258df250963d11dc) refactor: introduce expected pattern for error handling in master service - [`c34f5c4`](https://github.com/kvcache-ai/Mooncake/commit/c34f5c4fc1b71f3facff677a482c2449ffa3079f) Refactor error handling to use expected<T,E> pattern instead of ErrorCode - [`0302716`](https://github.com/kvcache-ai/Mooncake/commit/0302716bedb8a2ed7029a8cc3b50ffd2efa674e8) fix test compile - [`a61e14d`](https://github.com/kvcache-ai/Mooncake/commit/a61e14d20f1298303d6f5a0d501c5e4683b8ccbe) refactor: update client implementation and remove master.proto - [`a2005d2`](https://github.com/kvcache-ai/Mooncake/commit/a2005d2669daa5223c320ceb9d49c6957420c2d3) fix: update Python integration to work with new batch API - [`a9f1a5d`](https://github.com/kvcache-ai/Mooncake/commit/a9f1a5d8519aa67624a4e59b435366285374393f) refine master log and metric - [`3d0196f`](https://github.com/kvcache-ai/Mooncake/commit/3d0196f77ae20bc15575c0bba118fb6fb1f6af14) fix(ci): should alloc first - [`4cd8851`](https://github.com/kvcache-ai/Mooncake/commit/4cd8851e9f3404f86b249a41e54a33bb26933cfc) Merge branch 'main' into master_expected - [`2ae4bb6`](https://github.com/kvcache-ai/Mooncake/commit/2ae4bb6914393a82d7964f07d1baf31681def344) merge main - [`de2d387`](https://github.com/kvcache-ai/Mooncake/commit/de2d38757998a2fe646123c8606f0a9a7ccb20aa) fix tests ### 📊 Changes **25 files changed** (+2814 additions, -2119 deletions) <details> <summary>View changed files</summary> 📝 `.gitignore` (+3 -0) 📝 `mooncake-integration/store/store_py.cpp` (+296 -187) 📝 `mooncake-integration/store/store_py.h` (+7 -2) 📝 `mooncake-store/include/cachelib_memory_allocator/Slab.h` (+1 -1) 📝 `mooncake-store/include/client.h` (+69 -60) 📝 `mooncake-store/include/master_client.h` (+53 -48) 📝 `mooncake-store/include/master_metric_manager.h` (+36 -0) 📝 `mooncake-store/include/master_service.h` (+61 -50) ➕ `mooncake-store/include/rpc_helper.h` (+60 -0) 📝 `mooncake-store/include/rpc_service.h` (+299 -366) 📝 `mooncake-store/include/types.h` (+4 -3) 📝 `mooncake-store/include/utils.h` (+73 -1) 📝 `mooncake-store/include/utils/scoped_vlog_timer.h` (+37 -6) ➖ `mooncake-store/proto/master.proto` (+0 -225) 📝 `mooncake-store/src/client.cpp` (+650 -366) 📝 `mooncake-store/src/master_client.cpp` (+229 -263) 📝 `mooncake-store/src/master_metric_manager.cpp` (+106 -0) 📝 `mooncake-store/src/master_service.cpp` (+151 -161) 📝 `mooncake-store/tests/client_integration_test.cpp` (+129 -75) 📝 `mooncake-store/tests/e2e/client_wrapper.cpp` (+21 -13) _...and 5 more files_ </details> ### 📄 Description ## Overview This PR introduces the `tl::expected` pattern for error handling in the master service, replacing traditional `ErrorCode` return types with a more type-safe and explicit approach. ## Changes ### Core Improvements - **Type-safe error handling**: Replace `ErrorCode` return types with `tl::expected<T, ErrorCode>` pattern - **Clearer API semantics**: Success values and error codes are now explicitly separated - **Better developer experience**: - Success cases: access via `.value()` - Error cases: access via `.error()` - Eliminates ambiguity between success and error states ### Modified Components - **MasterService methods**: Updated to return `expected<void, ErrorCode>` or `expected<T, ErrorCode>` - **RPC service interfaces**: Modified to support the expected pattern - **Test suites**: Updated all related tests to handle new expected return types - **Dependencies**: Added necessary includes for `ylt/util/expected.hpp` ### Files Changed - `mooncake-store/include/master_service.h` - Updated method signatures - `mooncake-store/include/rpc_service.h` - RPC interface adaptations - `mooncake-store/include/types.h` - Minor formatting improvements - `mooncake-store/src/master_service.cpp` - Implementation updates - `mooncake-store/tests/` - Comprehensive test updates ## Benefits 1. **Type Safety**: Compile-time guarantees about error handling 2. **Explicit Error Handling**: No more ambiguous return codes 3. **Better Maintainability**: Clear separation of success and error paths 4. **Future-Ready**: Foundation for extending expected pattern to RPC responses ## Future Work This PR lays the groundwork for: - Extending expected pattern to RPC response types - Enhancing the error code system for more comprehensive error handling - Further improving type safety across the codebase ## Testing All existing tests have been updated and continue to pass, ensuring backward compatibility and correctness of the new error handling approach. --- **Note**: This change maintains API compatibility while improving error handling semantics. The expected pattern provides a modern, type-safe approach to error handling that will benefit future development and maintenance. --- <sub>🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.</sub>

bsakaguchi

2026-10-29 21:29:13 +00:00