You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@brpc.apache.org by "wasphin (via GitHub)" <gi...@apache.org> on 2026/09/19 05:51:18 UTC
[PR] Stabilize butex join test (brpc)
wasphin opened a new pull request, #3554:
URL: https://github.com/apache/brpc/pull/3554
### What problem does this PR solve?
Issue Number: resolve
Problem Summary:
`ButexTest.join` required each sleeping worker to finish within 15 ms of an
expected wall-clock time. Normal scheduling delays on a busy CI runner could
exceed that window even though every join completed successfully.
### What is changed and the side effects?
Changed:
- Validate that sleeps do not return early.
- Verify that successful joins observe completed target tasks.
- Preserve coverage for multiple and repeated joins without asserting a
narrow scheduling window.
Side effects:
- Performance effects: None.
- Breaking backward compatibility: None.
---
### Check List:
- Please make sure your changes are compilable.
- When providing us with a new feature, it is best to add related tests.
- Please follow [Contributor Covenant Code of Conduct](https://github.com/apache/brpc/blob/master/CODE_OF_CONDUCT.md).
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@brpc.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@brpc.apache.org
For additional commands, e-mail: dev-help@brpc.apache.org
Re: [PR] Stabilize butex join test (brpc)
Posted by "wasphin (via GitHub)" <gi...@apache.org>.
wasphin commented on code in PR #3554:
URL: https://github.com/apache/brpc/pull/3554#discussion_r4052733827
##########
test/bthread_butex_unittest.cpp:
##########
@@ -45,25 +45,36 @@ TEST(ButexTest, wait_on_already_timedout_butex) {
ASSERT_EQ(ETIMEDOUT, errno);
}
+struct JoinSleepArg {
+ bthread_t tid = 0;
+ uint64_t sleep_us = 0;
+ butil::atomic<bool> finished{false};
+};
+
void* sleeper(void* arg) {
bthread_usleep((uint64_t)arg);
return nullptr;
}
+void* join_sleeper(void* arg) {
+ JoinSleepArg* a = static_cast<JoinSleepArg*>(arg);
+ butil::Timer tm;
+ tm.start();
+ EXPECT_EQ(0, bthread_usleep(a->sleep_us));
+ tm.stop();
+ EXPECT_GE(tm.u_elapsed(), static_cast<int64_t>(a->sleep_us));
Review Comment:
Fixed by allowing the one-microsecond quantization introduced when Timer::u_elapsed() floors nanoseconds. The join test still rejects an early sleep beyond that rounding boundary, and it passes 20 repeated runs.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@brpc.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@brpc.apache.org
For additional commands, e-mail: dev-help@brpc.apache.org
Re: [PR] Stabilize butex join test (brpc)
Posted by "wwbmmm (via GitHub)" <gi...@apache.org>.
wwbmmm merged PR #3554:
URL: https://github.com/apache/brpc/pull/3554
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@brpc.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@brpc.apache.org
For additional commands, e-mail: dev-help@brpc.apache.org
Re: [PR] Stabilize butex join test (brpc)
Posted by "Copilot (via GitHub)" <gi...@apache.org>.
Copilot commented on code in PR #3554:
URL: https://github.com/apache/brpc/pull/3554#discussion_r4052705970
##########
test/bthread_butex_unittest.cpp:
##########
@@ -45,25 +45,36 @@ TEST(ButexTest, wait_on_already_timedout_butex) {
ASSERT_EQ(ETIMEDOUT, errno);
}
+struct JoinSleepArg {
+ bthread_t tid = 0;
+ uint64_t sleep_us = 0;
+ butil::atomic<bool> finished{false};
+};
+
void* sleeper(void* arg) {
bthread_usleep((uint64_t)arg);
return nullptr;
}
+void* join_sleeper(void* arg) {
+ JoinSleepArg* a = static_cast<JoinSleepArg*>(arg);
+ butil::Timer tm;
+ tm.start();
+ EXPECT_EQ(0, bthread_usleep(a->sleep_us));
+ tm.stop();
+ EXPECT_GE(tm.u_elapsed(), static_cast<int64_t>(a->sleep_us));
Review Comment:
This lower-bound check is still susceptible to a boundary flake: normal `bthread_usleep` schedules an absolute deadline after converting it to integer microseconds, while `Timer::u_elapsed()` floors nanoseconds to microseconds. A valid 100000-us sleep can therefore be observed as 99999 us when the fractional microsecond is lost, causing this assertion to fail intermittently. Compare with the same clock or account for the one-microsecond quantization.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@brpc.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@brpc.apache.org
For additional commands, e-mail: dev-help@brpc.apache.org