Skip to content

Commit b010b53

Browse files
committed
Default queue_size to the concurrency n when not configured
With a fixed default of 10, running with a higher --n left most of the thread pool idle (effective parallelism is min(n, queue_size)). An unset queue_size now resolves to n at processing time, so the pool is never starved by the default. An explicit config or constructor value still takes precedence.
1 parent 849ec67 commit b010b53

3 files changed

Lines changed: 28 additions & 6 deletions

File tree

Readme.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -403,7 +403,7 @@ settings.
403403
| Parameter | Description | Default |
404404
|-----------------|------------------------------------------------------------------------------------------------------------------|-------------------------|
405405
| `grobid_server` | GROBID server URL | `http://localhost:8070` |
406-
| `queue_size` | Number of files queued per processing chunk. **Tune carefully: a large queue size will result in the data being written less frequently** | 1000 |
406+
| `queue_size` | Number of files queued per processing chunk. If not set, it follows the concurrency `n` so the thread pool is never starved. **Tune carefully: a large queue size will result in the data being written less frequently, and on the archive/S3 paths a whole chunk is held in memory at once** | same as `n` |
407407
| `sleep_time` | Wait time when server is busy (seconds) | 5 |
408408
| `timeout` | Client-side timeout (seconds) | 180 |
409409
| `coordinates` | XML elements for coordinate extraction | See above |

grobid_client/grobid_client.py

Lines changed: 14 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -91,7 +91,9 @@ class GrobidClient(ApiClient):
9191
# Default configuration values
9292
DEFAULT_CONFIG: dict = {
9393
'grobid_server': 'http://localhost:8070',
94-
'queue_size': 10,
94+
# None means "follow the concurrency n at processing time", so the
95+
# default queue never starves the thread pool
96+
'queue_size': None,
9597
'sleep_time': 5,
9698
'timeout': 180,
9799
'coordinates': [
@@ -161,6 +163,14 @@ def _set_config_params(self, params: dict) -> None:
161163
if value is not None:
162164
self.config[key] = value
163165

166+
def _effective_queue_size(self, n: int) -> int:
167+
"""Return the configured queue_size, defaulting to the concurrency n.
168+
169+
A queue smaller than the thread pool leaves workers idle, so when no
170+
explicit value is configured the chunk size follows n.
171+
"""
172+
return self.config.get("queue_size") or n
173+
164174
def _warn_on_consolidation_timeout(self, consolidate_citations: bool) -> None:
165175
"""Warn when citation consolidation is enabled with a low client timeout.
166176
@@ -859,7 +869,7 @@ def _run_file_batches(
859869
860870
Returns the aggregated (processed, errors, skipped) counts.
861871
"""
862-
queue_size = self.config["queue_size"]
872+
queue_size = self._effective_queue_size(n)
863873
processed_files_count = 0
864874
errors_files_count = 0
865875
skipped_files_count = 0
@@ -1117,7 +1127,7 @@ def _process_archive_core(
11171127
Does not print the final summary (the caller does), so it can be
11181128
aggregated with other inputs when resolving a glob pattern.
11191129
"""
1120-
queue_size = self.config["queue_size"]
1130+
queue_size = self._effective_queue_size(n)
11211131

11221132
# Results must survive the temporary extraction directories, so when no
11231133
# output is given we default to a directory named after the archive. For
@@ -1278,7 +1288,7 @@ def _process_remote_files(
12781288
if output is None:
12791289
output = "."
12801290

1281-
queue_size = self.config["queue_size"]
1291+
queue_size = self._effective_queue_size(n)
12821292
print(f"Found {total} remote file(s) to process")
12831293
processed_count = 0
12841294
error_count = 0

tests/test_grobid_client.py

Lines changed: 13 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -43,12 +43,24 @@ def test_init_default_values(self, mock_configure_logging, mock_test_server):
4343
client = GrobidClient(check_server=False)
4444

4545
assert client.config['grobid_server'] == 'http://localhost:8070'
46-
assert client.config['queue_size'] == 10
46+
assert client.config['queue_size'] is None
4747
assert client.config['sleep_time'] == 5
4848
assert client.config['timeout'] == 180
4949
assert 'persName' in client.config['coordinates']
5050
mock_configure_logging.assert_called_once()
5151

52+
@patch('grobid_client.grobid_client.GrobidClient._test_server_connection')
53+
@patch('grobid_client.grobid_client.GrobidClient._configure_logging')
54+
def test_effective_queue_size(self, mock_configure_logging, mock_test_server):
55+
"""Test that an unset queue_size defaults to the concurrency n."""
56+
mock_test_server.return_value = (True, 200)
57+
58+
client = GrobidClient(check_server=False)
59+
assert client._effective_queue_size(40) == 40
60+
61+
client.config['queue_size'] = 100
62+
assert client._effective_queue_size(40) == 100
63+
5264
@patch('grobid_client.grobid_client.GrobidClient._test_server_connection')
5365
@patch('grobid_client.grobid_client.GrobidClient._configure_logging')
5466
def test_init_custom_values(self, mock_configure_logging, mock_test_server):

0 commit comments

Comments
 (0)