Skip to content

Commit 81a5924

Browse files
Merge branch 'main' into feature/vector-range-search
2 parents 763a1ec + 147b0b2 commit 81a5924

13 files changed

Lines changed: 143 additions & 97 deletions

.github/release-branches.txt

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,4 @@
1+
main
2+
1.0
3+
1.1
4+
1.2

.github/workflows/integration_tests-asan.yml

Lines changed: 7 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -1,15 +1,9 @@
11
name: Integration tests ASAN
22

33
on:
4-
pull_request: # This triggers the workflow for pull requests
5-
branches:
6-
- main # Run tests for pull requests targeting the "main" branch
7-
- fulltext
4+
pull_request:
85
push:
9-
branches:
10-
- main # Optional: Run tests for direct pushes to "main"
11-
- fulltext
12-
workflow_dispatch: # allow manual triggering
6+
workflow_dispatch:
137

148
concurrency:
159
group: integration-tests-asan-${{ github.head_ref || github.ref }}
@@ -24,6 +18,11 @@ jobs:
2418
- name: Checkout Code
2519
uses: actions/checkout@v3
2620

21+
- name: Verify Branch
22+
env:
23+
BRANCH: ${{ github.base_ref || github.ref_name }}
24+
run: grep -qxF "$BRANCH" .github/release-branches.txt
25+
2726
# Set up Docker to build and run the container
2827
- name: Set up Docker
2928
uses: docker/setup-buildx-action@v2

.github/workflows/integration_tests.yml

Lines changed: 7 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -1,15 +1,9 @@
11
name: Integration tests
22

33
on:
4-
pull_request: # This triggers the workflow for pull requests
5-
branches:
6-
- main # Run tests for pull requests targeting the "main" branch
7-
- fulltext
4+
pull_request:
85
push:
9-
branches:
10-
- main # Optional: Run tests for direct pushes to "main"
11-
- fulltext
12-
workflow_dispatch: # allow manual triggering
6+
workflow_dispatch:
137

148
concurrency:
159
group: integration-tests-${{ github.head_ref || github.ref }}
@@ -24,6 +18,11 @@ jobs:
2418
- name: Checkout Code
2519
uses: actions/checkout@v3
2620

21+
- name: Verify Branch
22+
env:
23+
BRANCH: ${{ github.base_ref || github.ref_name }}
24+
run: grep -qxF "$BRANCH" .github/release-branches.txt
25+
2726
# Set up Docker to build and run the container
2827
- name: Set up Docker
2928
uses: docker/setup-buildx-action@v2

.github/workflows/macos.yml

Lines changed: 7 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1,13 +1,9 @@
11
name: unittests-macos
22

33
on:
4-
pull_request: # This triggers the workflow for pull requests
5-
branches:
6-
- main # Run tests for pull requests targeting the "main" branch
4+
pull_request:
75
push:
8-
branches:
9-
- main # Optional: Run tests for direct pushes to "main"
10-
workflow_dispatch: # allow manual triggering
6+
workflow_dispatch:
117

128
concurrency:
139
group: unittests-macos-${{ github.head_ref || github.ref }}
@@ -22,6 +18,11 @@ jobs:
2218
- name: Checkout Code
2319
uses: actions/checkout@v3
2420

21+
- name: Verify Branch
22+
env:
23+
BRANCH: ${{ github.base_ref || github.ref_name }}
24+
run: grep -qxF "$BRANCH" .github/release-branches.txt
25+
2526
# Set up Docker to build and run the container
2627
- name: Build Code
2728
run: ./build.sh

.github/workflows/unittests-asan.yml

Lines changed: 7 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -1,15 +1,9 @@
11
name: unittests-asan
22

33
on:
4-
pull_request: # This triggers the workflow for pull requests
5-
branches:
6-
- main # Run tests for pull requests targeting the "main" branch
7-
- fulltext
4+
pull_request:
85
push:
9-
branches:
10-
- main # Optional: Run tests for direct pushes to "main"
11-
- fulltext
12-
workflow_dispatch: # allow manual triggering
6+
workflow_dispatch:
137

148
concurrency:
159
group: unittests-asan-${{ github.head_ref || github.ref }}
@@ -24,6 +18,11 @@ jobs:
2418
- name: Checkout Code
2519
uses: actions/checkout@v3
2620

21+
- name: Verify Branch
22+
env:
23+
BRANCH: ${{ github.base_ref || github.ref_name }}
24+
run: grep -qxF "$BRANCH" .github/release-branches.txt
25+
2726
# Set up Docker to build and run the container
2827
- name: Set up Docker
2928
uses: docker/setup-buildx-action@v2

.github/workflows/unittests-tsan.yml

Lines changed: 7 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -1,15 +1,9 @@
11
name: unittests-tsan
22

33
on:
4-
pull_request: # This triggers the workflow for pull requests
5-
branches:
6-
- main # Run tests for pull requests targeting the "main" branch
7-
- fulltext
4+
pull_request:
85
push:
9-
branches:
10-
- main # Optional: Run tests for direct pushes to "main"
11-
- fulltext
12-
workflow_dispatch: # allow manual triggering
6+
workflow_dispatch:
137

148
concurrency:
159
group: unittests-tsan-${{ github.head_ref || github.ref }}
@@ -24,6 +18,11 @@ jobs:
2418
- name: Checkout Code
2519
uses: actions/checkout@v3
2620

21+
- name: Verify Branch
22+
env:
23+
BRANCH: ${{ github.base_ref || github.ref_name }}
24+
run: grep -qxF "$BRANCH" .github/release-branches.txt
25+
2726
# Set up Docker to build and run the container
2827
- name: Set up Docker
2928
uses: docker/setup-buildx-action@v2

.github/workflows/unittests.yml

Lines changed: 7 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -1,15 +1,9 @@
11
name: unittests
22

33
on:
4-
pull_request: # This triggers the workflow for pull requests
5-
branches:
6-
- main # Run tests for pull requests targeting the "main" branch
7-
- fulltext
4+
pull_request:
85
push:
9-
branches:
10-
- main # Optional: Run tests for direct pushes to "main"
11-
- fulltext
12-
workflow_dispatch: # allow manual triggering
6+
workflow_dispatch:
137

148
concurrency:
159
group: unittests-${{ github.head_ref || github.ref }}
@@ -24,6 +18,11 @@ jobs:
2418
- name: Checkout Code
2519
uses: actions/checkout@v3
2620

21+
- name: Verify Branch
22+
env:
23+
BRANCH: ${{ github.base_ref || github.ref_name }}
24+
run: grep -qxF "$BRANCH" .github/release-branches.txt
25+
2726
# Set up Docker to build and run the container
2827
- name: Set up Docker
2928
uses: docker/setup-buildx-action@v2

COMPATIBILITY.md

Lines changed: 8 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -157,12 +157,16 @@ A fix for a compatibility bug released in a minor or patch release selectively p
157157

158158
It may be judged that a compatibility defect cannot reasonably be fixed while preserving the old behavior. In this case, the fix cannot be made until the next major release and will ignore the `search.emulate-release` mechanism. In other words, fixes made under this clause cannot be undone using the `search.emulate-release` override.
159159

160-
When feasible the old (non-compatible) behavior will be preserved for _at least_ one additional major release. If a bug was fixed in 1.x.x, then the 2.y.y will support emulating the 1.x.x release. However support in the 3. release or later releases is not ensured. Similar to the above clause, if retaining the parallel behavior becomes unreasonable to support, then it can be removed on the next major version.
160+
### Sunsetting of Incompatible Behavior
161+
162+
When feasible the old (non-compatible) behavior will be preserved for _at least_ one additional major release. If a bug was fixed in 1.x.x, then the 2.y.y will support emulating the 1.x.x release. However support in the 3. release or later releases is not ensured. Similar to the above clause, if retaining the parallel behavior becomes unreasonable to support, then it can be removed on the next major version (see the release notes for that release).
163+
164+
To ease application migration, incompatible behavior controlled by `search.emulate-release` is tracked by INFO fields. These fields count the number of uses of incompatible behavior by an application.
161165

162166
### Known Compatibility Defect Corrections
163167

164168
A list of the compatibility issues that have been fixed.
165169

166-
| Release | Description |
167-
| ------------------ | ----------- |
168-
| under construction | n/a |
170+
| Release | INFO field | Old Behavior | New Behavior |
171+
| ------------------ | ---------- | ------------ | ------------ |
172+
| under construction | n/a | --- | --- |
-77.3 MB
Binary file not shown.

vmsdk/src/thread_pool.cc

Lines changed: 72 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -20,9 +20,13 @@
2020
#include "absl/functional/any_invocable.h"
2121
#include "absl/log/check.h"
2222
#include "absl/status/status.h"
23+
#include "absl/strings/str_join.h"
2324
#include "absl/synchronization/blocking_counter.h"
2425
#include "absl/synchronization/mutex.h"
26+
#include "absl/time/clock.h"
27+
#include "absl/time/time.h"
2528
#include "status/status_macros.h"
29+
#include "vmsdk/src/log.h"
2630
#include "vmsdk/src/module_config.h"
2731

2832
namespace {
@@ -135,16 +139,44 @@ void ThreadPool::JoinWorkers() {
135139
suspend_workers_ = false;
136140
}
137141

138-
threads_.ClearWithCallback(
139-
[](auto thread) { pthread_join(thread->thread_id, nullptr); });
142+
// Wait up to 5s for every worker in threads_ to flag itself joinable.
143+
const absl::Time deadline = absl::Now() + absl::Seconds(5);
144+
auto count_unjoinable = [this]() {
145+
return threads_.CountIf(
146+
[](const std::shared_ptr<Thread> &t) { return !t->IsJoinable(); });
147+
};
148+
while (count_unjoinable() > 0 && absl::Now() < deadline) {
149+
absl::SleepFor(absl::Milliseconds(10));
150+
}
151+
if (count_unjoinable() > 0) {
152+
std::vector<std::string> hung;
153+
threads_.ForEach([&hung](const std::shared_ptr<Thread> &t) {
154+
if (!t->IsJoinable()) {
155+
hung.push_back(t->name);
156+
}
157+
});
158+
VMSDK_LOG(WARNING, nullptr)
159+
<< "ThreadPool shutdown timed out after 5s waiting for workers to exit;"
160+
<< " hung threads: " << absl::StrJoin(hung, ", ");
161+
CHECK(false) << "ThreadPool shutdown timeout: " << hung.size()
162+
<< " worker(s) did not become joinable within 5s";
163+
}
140164
started_ = false;
141-
142165
JoinTerminatedWorkers();
166+
CHECK(threads_.IsEmpty())
167+
<< "threads_ not empty after JoinTerminatedWorkers in JoinWorkers";
143168
}
144169

145170
void ThreadPool::JoinTerminatedWorkers() {
146-
pending_join_threads_.ClearWithCallback(
147-
[](auto thread) { pthread_join(thread->thread_id, nullptr); });
171+
while (true) {
172+
auto thread = threads_.PopIf(
173+
[](const std::shared_ptr<Thread> &t) { return t->IsJoinable(); });
174+
if (!thread.has_value()) {
175+
break;
176+
}
177+
// pthread_join intentionally runs with no ThreadSafeVector mutex held.
178+
pthread_join((*thread)->thread_id, nullptr);
179+
}
148180
}
149181

150182
absl::Status ThreadPool::SuspendWorkers() {
@@ -218,20 +250,15 @@ void ThreadPool::WorkerThread(std::shared_ptr<Thread> thread) {
218250
while (!condition.Eval()) {
219251
condition_.WaitWithTimeout(&queue_mutex_, absl::Seconds(1));
220252
if (thread->IsShutdown()) {
221-
thread->InvokeShutdownCallback();
222-
// remove this thread from the threads list and place it in the
223-
// pending join list
224-
threads_.PopIf([thread](std::shared_ptr<Thread> t) {
225-
return t->thread_id == thread->thread_id;
226-
});
227-
pending_join_threads_.Add(thread);
253+
thread->MarkJoinable();
228254
return;
229255
}
230256
}
231257
if (stop_mode_.has_value() &&
232258
(stop_mode_.value() == StopMode::kAbrupt ||
233259
std::all_of(priority_tasks_.begin(), priority_tasks_.end(),
234260
[](const auto &tasks) { return tasks.empty(); }))) {
261+
thread->MarkJoinable();
235262
return;
236263
}
237264
if (suspend_workers_) {
@@ -261,33 +288,52 @@ void ThreadPool::IncrThreadCountBy(size_t count) {
261288
std::shared_ptr<Thread> thread_ptr = std::make_shared<Thread>();
262289
ThreadRunContext *context = new ThreadRunContext{this, thread_ptr};
263290
pthread_create(&thread_ptr->thread_id, nullptr, RunWorkerThread, context);
264-
#ifndef __APPLE__
265291
size_t thread_num = threads_.Size();
266-
pthread_setname_np(thread_ptr->thread_id,
267-
(name_prefix_ + std::to_string(thread_num)).c_str());
292+
thread_ptr->name = name_prefix_ + std::to_string(thread_num);
293+
#ifndef __APPLE__
294+
pthread_setname_np(thread_ptr->thread_id, thread_ptr->name.c_str());
268295
#endif
269296
threads_.Add(thread_ptr);
270297
}
271298
}
272299

273300
void ThreadPool::DecrThreadCountBy(size_t count, bool sync) {
274-
auto threads = threads_.PopBackMulti(count);
275-
absl::BlockingCounter counter{static_cast<int>(threads.size())};
276-
for (const auto &thread : threads) {
277-
if (sync) {
278-
thread->Shutdown([&counter]() {
279-
counter.DecrementCount();
280-
}); // signal the thread to exit
281-
} else {
282-
thread->Shutdown();
301+
// Don't pop: leave the targeted threads in threads_ so JoinTerminatedWorkers
302+
// can reap them via the joinable_flag once they actually exit. We pick the
303+
// *last* `count` threads that are still active (not already shutdown).
304+
std::vector<std::shared_ptr<Thread>> targets;
305+
threads_.ForEach([&targets](const std::shared_ptr<Thread> &t) {
306+
if (!t->IsShutdown()) {
307+
targets.push_back(t);
283308
}
309+
});
310+
if (targets.size() > count) {
311+
targets.erase(targets.begin(), targets.end() - count);
312+
}
313+
for (const auto &thread : targets) {
314+
thread->Shutdown();
315+
}
316+
// Wake idle workers so they observe shutdown_flag without waiting 1s.
317+
{
318+
absl::MutexLock lock(&queue_mutex_);
319+
condition_.SignalAll();
284320
}
285-
286321
if (sync) {
287-
counter.Wait();
322+
for (const auto &thread : targets) {
323+
while (!thread->IsJoinable()) {
324+
absl::SleepFor(absl::Milliseconds(1));
325+
}
326+
}
327+
JoinTerminatedWorkers();
288328
}
289329
}
290330

331+
size_t ThreadPool::Size() const {
332+
return threads_.CountIf([](const std::shared_ptr<Thread> &t) {
333+
return !t->IsShutdown() && !t->IsJoinable();
334+
});
335+
}
336+
291337
void ThreadPool::Resize(size_t count, bool wait_for_resize) {
292338
size_t current_size = Size();
293339
if (count == current_size) {

0 commit comments

Comments
 (0)