Skip to content

refactor: refactor the entire file organization structure.#46

Open
magicheng0816 wants to merge 1 commit intojd-opensource:mainfrom
magicheng0816:refactor_file_structure
Open

refactor: refactor the entire file organization structure.#46
magicheng0816 wants to merge 1 commit intojd-opensource:mainfrom
magicheng0816:refactor_file_structure

Conversation

@magicheng0816
Copy link
Collaborator

No description provided.

Copy link

@gemini-code-assist gemini-code-assist bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

This pull request introduces a large-scale refactoring of the service's file organization structure. The changes are extensive, involving renaming of core components (Scheduler to Executor, http_service to api_service, rpc_service to instance_service, and load_balance_policy to scheduling_policy), moving files to a more modular directory structure, and updating CMakeLists.txt files and include paths accordingly. The refactoring appears to be consistent and well-executed. I've identified one critical issue related to graceful shutdown that was present in the old code and has been carried over to the new structure. Fixing it will improve the service's robustness.

Comment on lines +144 to +147
void shutdown_handler(int signal) {
LOG(WARNING) << "Received signal " << signal << ", stopping master...";
exit(1);
}

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

critical

The current shutdown_handler calls exit(1), which terminates the process immediately and prevents graceful shutdown. This can lead to resource leaks or inconsistent state, as cleanup code like master.stop() is never executed. To enable graceful shutdown, the signal handler should set an atomic flag to signal the main loop to terminate. This allows master.stop() to be called, which in turn ensures that server threads are properly joined.

Suggested change
void shutdown_handler(int signal) {
LOG(WARNING) << "Received signal " << signal << ", stopping master...";
exit(1);
}
void shutdown_handler(int signal) {
LOG(WARNING) << "Received signal " << signal << ", stopping master...";
g_signal_received.store(1, std::memory_order_relaxed);
}

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant