-
Notifications
You must be signed in to change notification settings - Fork 183
Add udp2, an experimental IPC layer implementation #1357
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
|
Awesome! Could you please share some information about the architecture of the new solution? I think there might be an original (acmsigmod?) article on how to build an effective interconnect layer. (I see some familiar elements in the code, but I cannot fully comprehend what it is) |
During performance testing, we identified the Motion node as a significant performance bottleneck. Further analysis revealed that the root cause stems from inefficiencies in the underlying IPC layer protocol implementation. To address this, we plan to enhance the IPC layer to improve overall system performance. Currently, the IPC layer is tightly coupled with the database kernel, which makes implementing performance optimizations challenging and limits flexibility. As an initial step, we are decoupling the IPC layer from the database kernel. This separation will not only simplify future improvements but also enable independent testing and development of the IPC layer. This commit represents foundational work—specifically, the decoupling of the existing UDP-based IPC layer from the database kernel—and lays the groundwork for subsequent performance optimization efforts. |
|
The following two files also need the license header: |
7e7c028 to
251adf4
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
+1 for ASF license check.
| static void | ||
| initMutex(pthread_mutex_t *mutex) | ||
| { | ||
| pthread_mutexattr_t m_atts; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
why not use std::mutex and std::lock_guard ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Both ic_control_info.lock and trans_proto_stats.lock are initialized using initMutex(). The usage of ic_control_info.lock is highly complex, and replacing it with std::mutex and std::lock_guard would be challenging in the near term. In contrast, trans_proto_stats.lock has a much simpler usage pattern, and in the latest commit, it has already been successfully migrated to std::mutex and std::lock_guard. As the protocol implementation continues to evolve and the usage of ic_control_info.lock becomes simpler, we plan to gradually replace it with modern C++ synchronization primitives as well.
fix in 30b949c
|
Could you please share some information about the architecture of the new solution by doc ? |
Describes the architecture and other aspects in README.md,see 94428f1 |
Fixes #ISSUE_Number
What does this PR do?
Type of Change
Breaking Changes
Test Plan
make installcheckmake -C src/test installcheck-cbdb-parallelImpact
Performance:
User-facing changes:
Dependencies:
Checklist
Additional Context
CI Skip Instructions