Skip to content

Commit fe34d68

Browse files
luoluo
authored andcommitted
fix(thread-pool): preserve binding defaults and honor CPU limits
1 parent d07345f commit fe34d68

15 files changed

Lines changed: 301 additions & 54 deletions

File tree

THREAD_POOL_BEHAVIOR.md

Lines changed: 42 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -57,7 +57,9 @@ This is a C++ source compatibility change. It intentionally turns ambiguous
5757
calls into compile errors instead of silently choosing the wrong behavior.
5858

5959
CPU binding is implemented on Linux excluding Android. On unsupported
60-
platforms, the binding flag does not change thread affinity.
60+
platforms, the binding flag does not change thread affinity. Linux binding
61+
enumerates the calling thread's allowed CPU mask, so non-zero-based and
62+
restricted cpusets are honored. Affinity API failures are logged.
6163

6264
## DB global query and optimize pools
6365

@@ -66,8 +68,8 @@ initialized independently from `CgroupUtil::getCpuLimit()`:
6668

6769
| Configuration | Default count | Default binding |
6870
|---|---:|---|
69-
| Query pool | Cgroup CPU limit | Disabled |
70-
| Optimize pool | Cgroup CPU limit | Disabled |
71+
| Query pool | Cgroup CPU limit | Enabled |
72+
| Optimize pool | Cgroup CPU limit | Enabled |
7173

7274
Therefore:
7375

@@ -76,6 +78,8 @@ Therefore:
7678
CPU capacity.
7779
- In a CPU-limited container, the DB defaults may differ from
7880
`std::thread::hardware_concurrency()`.
81+
- Cgroup v2 `cpu.max` values are parsed as `quota period`; `max period` is
82+
treated as unlimited and falls back to the online CPU count.
7983
- Query and optimize counts are independent. Setting `query_threads` does not
8084
implicitly change `optimize_threads`.
8185

@@ -85,7 +89,9 @@ runtime behavior was hardware-concurrency threads with binding enabled. This
8589
meant configured values such as `optimize_threads=1` were ignored.
8690

8791
After this fix, the configured count and binding are both forwarded
88-
explicitly, so the effective behavior matches the configuration.
92+
explicitly, so the effective behavior matches the configuration. Binding
93+
remains enabled by default to preserve the previous effective behavior; users
94+
can explicitly disable it when scheduler-managed placement is preferable.
8995

9096
## Python API
9197

@@ -106,21 +112,21 @@ Behavior:
106112
|---|---|
107113
| Count is `None` | Use the corresponding cgroup-derived DB default |
108114
| Count is a positive integer | Use exactly that many threads |
109-
| Binding is `None` | Use the DB default (`False`) |
115+
| Binding is `None` | Use the DB default (`True`) |
110116
| Binding is `False` | Do not bind worker threads |
111117
| Binding is `True` | Bind worker threads where supported |
112118

113119
Examples:
114120

115121
```python
116-
# Both pools use their cgroup-derived counts without binding.
122+
# Both pools use their cgroup-derived counts with binding.
117123
zvec.init()
118124

119-
# Optimize uses exactly one unbound worker.
125+
# Optimize uses exactly one bound worker.
120126
zvec.init(optimize_threads=1)
121127

122-
# Query uses four bound workers; optimize keeps its independent defaults.
123-
zvec.init(query_threads=4, query_thread_binding=True)
128+
# Explicitly let the OS schedule both pools without affinity pinning.
129+
zvec.init(query_thread_binding=False, optimize_thread_binding=False)
124130
```
125131

126132
## C API
@@ -134,7 +140,7 @@ zvec_config_data_set_optimize_thread_binding(config, binding);
134140
zvec_config_data_get_optimize_thread_binding(config);
135141
```
136142
137-
If these setters are not called, binding remains disabled.
143+
If these setters are not called, binding remains enabled.
138144
139145
## Internal call-site migration
140146
@@ -146,7 +152,8 @@ behavior:
146152
- Recall tools with no requested count still use hardware concurrency with
147153
binding.
148154
- Explicit benchmark and recall thread counts are now honored.
149-
- `Index::Merge` uses `write_concurrency` without binding.
155+
- `Index::Merge` uses `write_concurrency` with binding, preserving its previous
156+
effective affinity behavior.
150157
- DB query and optimize pools use their configured count and binding.
151158
- Existing call sites already passing both count and binding are unchanged.
152159
@@ -165,7 +172,10 @@ ThreadPool(threads, true);
165172
## Performance validation
166173
167174
The fix was validated with an end-to-end Python DB API benchmark using the
168-
SIFT dataset:
175+
SIFT dataset. The measured post-fix revision used disabled binding defaults.
176+
The current compatibility-preserving defaults keep binding enabled, so the
177+
post-fix placement measured below is reproduced by explicitly setting both
178+
binding options to `False`.
169179
170180
| Parameter | Value |
171181
|---|---|
@@ -192,7 +202,18 @@ The test machine exposes 64 logical CPUs. The benchmark called:
192202
zvec.init(optimize_threads=1)
193203
```
194204

195-
It did not override the query thread count or either binding option.
205+
It did not override the query thread count or either binding option. In that
206+
measured revision the effective binding defaults were `False`; with the
207+
current defaults, use the following equivalent configuration for the post-fix
208+
run:
209+
210+
```python
211+
zvec.init(
212+
optimize_threads=1,
213+
query_thread_binding=False,
214+
optimize_thread_binding=False,
215+
)
216+
```
196217

197218
| Pool / behavior | Before the fix | After the fix |
198219
|---|---|---|
@@ -301,13 +322,17 @@ Behavior is unchanged for:
301322
- `ThreadPool()`;
302323
- `ThreadPool(n, false)`;
303324
- `ThreadPool(n, true)`;
325+
- `Index::Merge` CPU binding;
326+
- DB query and optimize pool binding defaults;
304327
- internal call sites whose old behavior was already explicit and correct.
305328
306329
Behavior intentionally changes for:
307330
308331
- one-argument thread counts that were previously interpreted as booleans;
309332
- DB thread-count configuration that was previously ignored;
310-
- `Index::Merge` and recall resize paths affected by ambiguous or reversed
311-
arguments;
312-
- DB binding defaults, which now explicitly default to disabled instead of
313-
being accidentally enabled by overload resolution.
333+
- recall resize paths affected by reversed arguments.
334+
335+
DB binding remains enabled by default, matching the previous effective
336+
runtime behavior. Adding binding fields to the public C++ `ConfigData` struct
337+
changes its layout, so consumers linking across a C++ ABI boundary must rebuild
338+
against the updated headers.

python/tests/detail/test_db_config.py

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -157,6 +157,10 @@ def test_query_threads(self):
157157
def test_query_thread_binding(self):
158158
zvec.init(query_thread_binding=True)
159159

160+
@run_in_subprocess
161+
def test_query_thread_binding_disabled(self):
162+
zvec.init(query_thread_binding=False)
163+
160164
@run_in_subprocess
161165
def test_query_thread_binding_invalid(self):
162166
with pytest.raises(TypeError):
@@ -186,6 +190,10 @@ def test_optimize_threads(self):
186190
def test_optimize_thread_binding(self):
187191
zvec.init(optimize_thread_binding=True)
188192

193+
@run_in_subprocess
194+
def test_optimize_thread_binding_disabled(self):
195+
zvec.init(optimize_thread_binding=False)
196+
189197
@run_in_subprocess
190198
def test_optimize_thread_binding_invalid(self):
191199
with pytest.raises(TypeError):

python/zvec/zvec.py

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -83,14 +83,14 @@ def init(
8383
Must be ≥ 1 if provided.
8484
query_thread_binding (Optional[bool], optional):
8585
Whether to bind query worker threads to CPU cores. Defaults to
86-
Zvec's internal setting, currently ``False``.
86+
Zvec's internal setting, currently ``True``.
8787
optimize_threads (Optional[int], optional):
8888
Threads for background tasks (e.g., compaction, indexing).
8989
If ``None`` (default), inferred independently from available CPU
9090
cores (via cgroup).
9191
optimize_thread_binding (Optional[bool], optional):
9292
Whether to bind optimize worker threads to CPU cores. Defaults to
93-
Zvec's internal setting, currently ``False``.
93+
Zvec's internal setting, currently ``True``.
9494
invert_to_forward_scan_ratio (Optional[float], optional):
9595
Threshold to switch from inverted index to full forward scan.
9696
Range: [0.0, 1.0]. Higher → more aggressive index skipping.

src/ailego/parallel/thread_pool.cc

Lines changed: 45 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -12,33 +12,64 @@
1212
// See the License for the specific language governing permissions and
1313
// limitations under the License.
1414

15+
#include <zvec/ailego/logger/logger.h>
1516
#include <zvec/ailego/parallel/thread_pool.h>
1617

17-
#if (defined(__linux) || defined(__linux__)) && !defined(__ANDROID__)
18+
#if defined(__linux__) && !defined(__ANDROID__)
1819
#include <pthread.h>
1920

21+
static inline bool GetAllowedCpuMask(cpu_set_t *mask) {
22+
CPU_ZERO(mask);
23+
const int error = pthread_getaffinity_np(pthread_self(), sizeof(*mask), mask);
24+
if (error != 0) {
25+
LOG_WARN("Failed to get thread affinity mask, error[%d]", error);
26+
return false;
27+
}
28+
return true;
29+
}
30+
2031
static inline void BindThreads(std::vector<std::thread> &pool) {
21-
uint32_t hc = std::thread::hardware_concurrency();
22-
if (hc > 1) {
23-
cpu_set_t mask;
24-
25-
for (size_t i = 0u; i < pool.size(); ++i) {
26-
CPU_ZERO(&mask);
27-
CPU_SET(i % hc, &mask);
28-
pthread_setaffinity_np(pool[i].native_handle(), sizeof(mask), &mask);
32+
cpu_set_t allowed_mask;
33+
if (!GetAllowedCpuMask(&allowed_mask)) {
34+
return;
35+
}
36+
37+
std::vector<int> allowed_cpus;
38+
for (int cpu = 0; cpu < CPU_SETSIZE; ++cpu) {
39+
if (CPU_ISSET(cpu, &allowed_mask)) {
40+
allowed_cpus.push_back(cpu);
41+
}
42+
}
43+
if (allowed_cpus.empty()) {
44+
LOG_WARN("Cannot bind thread pool: affinity mask has no allowed CPUs");
45+
return;
46+
}
47+
48+
cpu_set_t target_mask;
49+
for (size_t i = 0u; i < pool.size(); ++i) {
50+
CPU_ZERO(&target_mask);
51+
CPU_SET(allowed_cpus[i % allowed_cpus.size()], &target_mask);
52+
const int error = pthread_setaffinity_np(pool[i].native_handle(),
53+
sizeof(target_mask), &target_mask);
54+
if (error != 0) {
55+
LOG_WARN("Failed to bind thread pool worker[%zu], error[%d]", i, error);
2956
}
3057
}
3158
}
3259

3360
static inline void UnbindThreads(std::vector<std::thread> &pool) {
3461
cpu_set_t mask;
3562
CPU_ZERO(&mask);
36-
37-
for (size_t i = 0u; i < CPU_SETSIZE; ++i) {
38-
CPU_SET(i, &mask);
63+
for (int cpu = 0; cpu < CPU_SETSIZE; ++cpu) {
64+
CPU_SET(cpu, &mask);
3965
}
66+
4067
for (size_t i = 0u; i < pool.size(); ++i) {
41-
pthread_setaffinity_np(pool[i].native_handle(), sizeof(mask), &mask);
68+
const int error =
69+
pthread_setaffinity_np(pool[i].native_handle(), sizeof(mask), &mask);
70+
if (error != 0) {
71+
LOG_WARN("Failed to unbind thread pool worker[%zu], error[%d]", i, error);
72+
}
4273
}
4374
}
4475
#else
@@ -130,4 +161,4 @@ bool ThreadPool::picking(ThreadPool::Task *task) {
130161
}
131162

132163
} // namespace ailego
133-
} // namespace zvec
164+
} // namespace zvec

src/core/interface/index.cc

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -986,7 +986,7 @@ int Index::Merge(const std::vector<Index::Pointer> &indexes,
986986
reducer->set_thread_pool(options.pool);
987987
} else {
988988
local_thread_pool =
989-
std::make_unique<ailego::ThreadPool>(options.write_concurrency, false);
989+
std::make_unique<ailego::ThreadPool>(options.write_concurrency, true);
990990
reducer->set_thread_pool(local_thread_pool.get());
991991
}
992992

src/db/common/cgroup_util.cc

Lines changed: 46 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,8 @@
1313
// limitations under the License.
1414

1515
#include "db/common/cgroup_util.h"
16+
#include <charconv>
17+
#include <system_error>
1618

1719
namespace zvec {
1820

@@ -114,19 +116,15 @@ bool CgroupUtil::readCpuCgroup() {
114116
// cgroup v2
115117
std::ifstream file("/sys/fs/cgroup/cpu.max");
116118
if (file.is_open()) {
117-
uint64_t quota, period;
118-
char slash;
119-
file >> quota >> slash >> period;
120-
file.close();
119+
std::string cpu_max;
120+
std::getline(file, cpu_max);
121121

122-
if (quota != std::numeric_limits<uint64_t>::max() && quota != 0 &&
123-
period > 0) {
124-
cpu_cores_ =
125-
static_cast<int>(std::ceil(static_cast<double>(quota) / period));
122+
int cpu_cores = 0;
123+
if (parseCpuMax(cpu_max, &cpu_cores)) {
124+
cpu_cores_ = cpu_cores;
126125
return true;
127-
} else {
128-
return false;
129126
}
127+
return false;
130128
}
131129

132130
// cgroup v1
@@ -150,6 +148,43 @@ bool CgroupUtil::readCpuCgroup() {
150148
return false;
151149
}
152150

151+
bool CgroupUtil::parseCpuMax(const std::string &cpu_max, int *cpu_cores) {
152+
if (cpu_cores == nullptr) {
153+
return false;
154+
}
155+
156+
std::istringstream stream(cpu_max);
157+
std::string quota_token;
158+
uint64_t period = 0;
159+
if (!(stream >> quota_token >> period) || period == 0) {
160+
return false;
161+
}
162+
163+
std::string trailing_token;
164+
if (stream >> trailing_token) {
165+
return false;
166+
}
167+
if (quota_token == "max") {
168+
return false;
169+
}
170+
171+
uint64_t quota = 0;
172+
const auto result = std::from_chars(
173+
quota_token.data(), quota_token.data() + quota_token.size(), quota);
174+
if (result.ec != std::errc{} ||
175+
result.ptr != quota_token.data() + quota_token.size() || quota == 0) {
176+
return false;
177+
}
178+
179+
const uint64_t cores = quota / period + (quota % period != 0 ? 1 : 0);
180+
if (cores > static_cast<uint64_t>(std::numeric_limits<int>::max())) {
181+
return false;
182+
}
183+
184+
*cpu_cores = static_cast<int>(cores);
185+
return true;
186+
}
187+
153188
void CgroupUtil::updateMemoryLimit() {
154189
if (readMemoryCgroup()) {
155190
return;
@@ -491,4 +526,4 @@ double CgroupUtil::calculateMacOSCpuUsage() {
491526
}
492527
#endif
493528

494-
} // namespace zvec
529+
} // namespace zvec

src/db/common/cgroup_util.h

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -45,6 +45,10 @@ class CgroupUtil {
4545
static int getCpuLimit();
4646
static uint64_t getMemoryLimit();
4747

48+
// Parse a cgroup v2 cpu.max value. Returns false for an unlimited or
49+
// malformed value so callers can fall back to the host CPU count.
50+
static bool parseCpuMax(const std::string &cpu_max, int *cpu_cores);
51+
4852
// Static methods to get other resources
4953
static double getCpuUsage();
5054
static uint64_t getMemoryUsage();
@@ -101,4 +105,4 @@ class CgroupUtil {
101105
#endif
102106
};
103107

104-
} // namespace zvec
108+
} // namespace zvec

0 commit comments

Comments
 (0)