This is an automated email from the ASF dual-hosted git repository.
leborchuk pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/cloudberry.git
The following commit(s) were added to refs/heads/main by this push:
new c80d6cf7b97 Fix putIntoUnackQueueRing function logic for Intercontect
interfaces (#1931)
c80d6cf7b97 is described below
commit c80d6cf7b9762370030b519db0b526aed228e9d2
Author: ZTE-EBASE <[email protected]>
AuthorDate: Tue Sep 1 04:25:18 2026 +0800
Fix putIntoUnackQueueRing function logic for Intercontect interfaces
(#1931)
Fix putIntoUnackQueueRing function logic for Intercontect interfaces
what
There are several logic issues in the putIntoUnackQueueRing function.
Defect 1: Redundant idx calculation (Severity: Low-Medium)
Defect 2: Misuse of TIMER_SPAN_LOSS (Severity: Medium)
Defect 3: Redundant conditional judgment (Severity: Very Low)
why
For Defect 1 (Redundant idx calculation):
Performance waste: Each call executes one extra integer division and modulo
operation (~5-10ns per call)
Poor maintainability: If someone modifies one formula but forgets the
other, it introduces hard-to-debug bugs
Code confusion: Readers wonder "why calculate twice?"
Misleading logs: Line 7054's log shows the first calculation, but line
7058's result is actually used
For Defect 2 (TIMER_SPAN_LOSS misuse):
Time alignment bias: Expected 5ms boundary alignment, actual 2.5ms alignment
Imprecise retransmission timing: May cause packets to trigger
retransmission too early or too late
Degraded flow control performance: Affects accuracy of timeout-based
mechanisms
Subtle bug: Not immediately obvious during testing but impacts long-term
stability
For Defect 3 (Redundant conditional):
---------
Co-authored-by: 王平10304955 <[email protected]>
---
contrib/interconnect/udp/ic_udpifc.c | 4 ++--
contrib/udp2/ic_common/udp2/ic_udp2.cpp | 2 +-
contrib/udp2/ic_common/udp2/ic_udp2_internal.hpp | 2 +-
3 files changed, 4 insertions(+), 4 deletions(-)
diff --git a/contrib/interconnect/udp/ic_udpifc.c
b/contrib/interconnect/udp/ic_udpifc.c
index 0f26d73f936..0f23ecaad8e 100644
--- a/contrib/interconnect/udp/ic_udpifc.c
+++ b/contrib/interconnect/udp/ic_udpifc.c
@@ -582,7 +582,7 @@ static ICGlobalControlInfo ic_control_info;
*/
#define UNACK_QUEUE_RING_SLOTS_NUM (2000)
#define TIMER_SPAN (Gp_interconnect_timer_period * 1000ULL) /* default: 5ms
*/
-#define TIMER_SPAN_LOSS (Gp_interconnect_timer_period * 500ULL) /*
default: 5ms */
+#define TIMER_SPAN_LOSS (Gp_interconnect_timer_period * 500ULL) /*
default: 2.5ms */
#define TIMER_CHECKING_PERIOD Gp_interconnect_timer_checking_period /*
default: 20ms */
#define UNACK_QUEUE_RING_LENGTH (UNACK_QUEUE_RING_SLOTS_NUM * TIMER_SPAN)
#define UNACK_QUEUE_RING_LENGTH_LOSS (UNACK_QUEUE_RING_SLOTS_NUM *
TIMER_SPAN_LOSS)
@@ -7033,7 +7033,7 @@ putIntoUnackQueueRing(UnackQueueRing *uqr, ICBuffer *buf,
uint64 expTime, uint64
else
{
if (uqr->currentTime == 0)
- uqr->currentTime = now - (now % TIMER_SPAN_LOSS);
+ uqr->currentTime = now - (now % TIMER_SPAN);
diff = now + expTime - uqr->currentTime;
if (diff >= UNACK_QUEUE_RING_LENGTH)
diff --git a/contrib/udp2/ic_common/udp2/ic_udp2.cpp
b/contrib/udp2/ic_common/udp2/ic_udp2.cpp
index a747d91f600..ca001646151 100644
--- a/contrib/udp2/ic_common/udp2/ic_udp2.cpp
+++ b/contrib/udp2/ic_common/udp2/ic_udp2.cpp
@@ -3055,7 +3055,7 @@ putIntoUnackQueueRing(UnackQueueRing *uqr, ICBuffer *buf,
uint64 expTime, uint64
else
{
if (uqr->currentTime == 0)
- uqr->currentTime = now - (now % TIMER_SPAN_LOSS);
+ uqr->currentTime = now - (now % TIMER_SPAN);
diff = now + expTime - uqr->currentTime;
if (diff >= UNACK_QUEUE_RING_LENGTH)
diff --git a/contrib/udp2/ic_common/udp2/ic_udp2_internal.hpp
b/contrib/udp2/ic_common/udp2/ic_udp2_internal.hpp
index 2602133a9e5..0fa95713735 100644
--- a/contrib/udp2/ic_common/udp2/ic_udp2_internal.hpp
+++ b/contrib/udp2/ic_common/udp2/ic_udp2_internal.hpp
@@ -909,7 +909,7 @@ struct ICGlobalControlInfo
*/
#define UNACK_QUEUE_RING_SLOTS_NUM (2000)
#define TIMER_SPAN (session_param.Gp_interconnect_timer_period * 1000ULL)
/* default: 5ms */
-#define TIMER_SPAN_LOSS (session_param.Gp_interconnect_timer_period * 500ULL)
/* default: 5ms */
+#define TIMER_SPAN_LOSS (session_param.Gp_interconnect_timer_period * 500ULL)
/* default: 2.5ms */
#define TIMER_CHECKING_PERIOD
(session_param.Gp_interconnect_timer_checking_period) /* default: 20ms */
#define UNACK_QUEUE_RING_LENGTH (UNACK_QUEUE_RING_SLOTS_NUM * TIMER_SPAN)
#define UNACK_QUEUE_RING_LENGTH_LOSS (UNACK_QUEUE_RING_SLOTS_NUM *
TIMER_SPAN_LOSS)
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
