Skip to content

Commit 43e2074

Browse files
committed
addressing PR comments
Signed-off-by: niranda perera <niranda.perera@gmail.com>
1 parent 08f07b6 commit 43e2074

File tree

2 files changed

+12
-5
lines changed

2 files changed

+12
-5
lines changed

cpp/include/rapidsmpf/shuffler/finish_counter.hpp

Lines changed: 9 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -51,10 +51,7 @@ class FinishCounter {
5151
*
5252
* @warning A callback must be fast and non-blocking and should not call any of the
5353
* `wait*` methods. And be very careful if acquiring locks. Ideally it should be used
54-
* to signal a separate thread to do the actual processing (eg. WaitHand).
55-
*
56-
* @note Caller needs to be careful when using both callbacks and wait* methods
57-
* together.
54+
* to signal a separate thread to do the actual processing.
5855
*/
5956
using FinishedCallback = std::function<void(PartID)>;
6057

@@ -123,6 +120,10 @@ class FinishCounter {
123120
* @throws std::out_of_range If all partitions have already been waited on.
124121
* @throws std::runtime_error If timeout was set and no partitions have been finished
125122
* by the expiration.
123+
*
124+
* @note The caller needs to be careful when using `wait_any` alongside `is_finished`.
125+
* For example, `is_finished()` will return true once all partitions have been
126+
* finished, regardless of how many partitions were waited on.
126127
*/
127128
PartID wait_any(std::optional<std::chrono::milliseconds> timeout = {});
128129

@@ -140,6 +141,10 @@ class FinishCounter {
140141
* @throws std::out_of_range If the desired partition is unavailable.
141142
* @throws std::runtime_error If timeout was set and requested partition has been
142143
* finished by the expiration.
144+
*
145+
* @note The caller needs to be careful when using `wait_on` alongside `is_finished`.
146+
* For example, `is_finished()` will return true once all partitions have been
147+
* finished, regardless of how many partitions were waited on.
143148
*/
144149
void wait_on(PartID pid, std::optional<std::chrono::milliseconds> timeout = {});
145150

cpp/src/shuffler/finish_counter.cpp

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -81,7 +81,9 @@ void FinishCounter::add_finished_chunk(PartID pid) {
8181
p_info.add_finished_chunk(nranks_);
8282

8383
if (p_info.is_finished(nranks_)) {
84-
assert(n_unfinished_partitions_ > 0); // TODO: use a debug flag
84+
RAPIDSMPF_EXPECTS(
85+
n_unfinished_partitions_ > 0, "all partitions have been finished"
86+
);
8587
n_unfinished_partitions_--;
8688
lock.unlock();
8789

0 commit comments

Comments
 (0)