From 5d0cf9f11f69dbb5aaa14a49b84fa773eb047ee2 Mon Sep 17 00:00:00 2001 From: Masaharu Hayashi Date: Tue, 20 Oct 2026 22:50:47 +0000 Subject: [PATCH] =?UTF-8?q?fix(invenio-oaiserver):=20scroll=20=E3=81=AE=20?= =?UTF-8?q?keep-alive=20=E6=97=A2=E5=AE=9A=E5=80=A4=E3=82=92=2024=20?= =?UTF-8?q?=E6=99=82=E9=96=93=E3=81=8B=E3=82=89=201=20=E6=99=82=E9=96=93?= =?UTF-8?q?=E3=81=AB=E7=9F=AD=E7=B8=AE=E3=81=99=E3=82=8B?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit OAISERVER_RESUMPTION_TOKEN_EXPIRE_TIME は resumptionToken の有効期限であると同時に、 ListRecords / ListIdentifiers が開く Elasticsearch scroll の keep-alive にも使われる。 scroll は最終ページを返したときにしか clear_scroll されないため、途中でやめた ハーベストの scroll コンテキストは keep-alive の間ヒープに残り続ける。既定値が 24 時間だったため、ES データノードのヒープが数日かけて増え続けていた。 keep-alive はページを取るたびに延長されるので、必要なのは「次のページを取りに 来るまでの間隔」をまかなう長さだけで、ハーベスト全体の所要時間ではない。 既定値を 3600 秒にし、実際の値と食い違っていた docstring(Default: 60 seconds)を直す。 Co-Authored-By: Claude Opus 5.5 --- .../invenio_oaiserver/config.py | 17 +++++++++++-- modules/invenio-oaiserver/tests/test_app.py | 15 +++++++++++ modules/invenio-oaiserver/tests/test_query.py | 25 +++++++++++++++++++ 3 files changed, 55 insertions(+), 2 deletions(-) diff --git a/modules/invenio-oaiserver/invenio_oaiserver/config.py b/modules/invenio-oaiserver/invenio_oaiserver/config.py index f9ba3e382d..f04edd35bf 100644 --- a/modules/invenio-oaiserver/invenio_oaiserver/config.py +++ b/modules/invenio-oaiserver/invenio_oaiserver/config.py @@ -35,15 +35,28 @@ with meanings as defined in ISO8601. """ -OAISERVER_RESUMPTION_TOKEN_EXPIRE_TIME = 24 * 60 * 60 +OAISERVER_RESUMPTION_TOKEN_EXPIRE_TIME = 60 * 60 """The expiration time of a resumption token in seconds. -**Default: 60 seconds = 1 minute**. +**Default: 3600 seconds = 1 hour**. + +The same value is used as the keep-alive of the Elasticsearch scroll +opened by ListRecords / ListIdentifiers. The keep-alive is renewed on +every page request, so it only needs to cover the interval between two +consecutive requests of a harvester, not the whole harvest. .. note:: Setting longer expiration time may have a negative impact on your Elasticsearch cluster as it might need to keep open cursors. + A scroll is cleared only when the last page is returned, so every + harvest abandoned halfway keeps its scroll contexts on the heap until + this time elapses. The previous default (24 hours) caused the heap of + the Elasticsearch data nodes to grow for days. + + Elasticsearch rejects a keep-alive longer than + ``search.max_keep_alive`` (default: 24h). Keep this value at or below + that setting. https://www.elastic.co/guide/en/elasticsearch/reference/current/search-request-scroll.html """ diff --git a/modules/invenio-oaiserver/tests/test_app.py b/modules/invenio-oaiserver/tests/test_app.py index 761cbafcea..fdf7a786b8 100644 --- a/modules/invenio-oaiserver/tests/test_app.py +++ b/modules/invenio-oaiserver/tests/test_app.py @@ -27,3 +27,18 @@ def test_init(): app = Flask('testapp') with pytest.warns(None): InvenioOAIServer(app) + + +def test_resumption_token_expire_time_default(): + """Scroll keep-alive defaults to 1 hour, not 24 hours.""" + app = Flask('testapp') + InvenioOAIServer(app) + assert app.config['OAISERVER_RESUMPTION_TOKEN_EXPIRE_TIME'] == 3600 + + +def test_resumption_token_expire_time_override(): + """A value set by the instance config is kept.""" + app = Flask('testapp') + app.config['OAISERVER_RESUMPTION_TOKEN_EXPIRE_TIME'] = 600 + InvenioOAIServer(app) + assert app.config['OAISERVER_RESUMPTION_TOKEN_EXPIRE_TIME'] == 600 diff --git a/modules/invenio-oaiserver/tests/test_query.py b/modules/invenio-oaiserver/tests/test_query.py index e62d2283db..43583aa219 100644 --- a/modules/invenio-oaiserver/tests/test_query.py +++ b/modules/invenio-oaiserver/tests/test_query.py @@ -460,3 +460,28 @@ def rq_spy(_from, _until): get_records(**kwargs) assert called["filter"] == expect_range assert called["rq"] == expect_rq + + +def test_get_records_scroll_keep_alive(es_app, db): + """The scroll keep-alive follows OAISERVER_RESUMPTION_TOKEN_EXPIRE_TIME.""" + assert es_app.config['OAISERVER_RESUMPTION_TOKEN_EXPIRE_TIME'] == 3600 + + # first page: the scroll is opened with the configured keep-alive + with patch.object(current_search_client, 'search', + wraps=current_search_client.search) as mock_search: + get_records() + assert mock_search.call_args[1]['scroll'] == '3600s' + + # next pages: the keep-alive is renewed with the same value + response = {'_scroll_id': 'dummy', 'hits': {'total': 0, 'hits': []}} + with patch.object(current_search_client, 'scroll', + return_value=response) as mock_scroll, \ + patch.object(current_search_client, 'clear_scroll'): + get_records(resumptionToken={'page': 2, 'scroll_id': 'dummy'}) + mock_scroll.assert_called_once_with(scroll_id='dummy', scroll='3600s') + + es_app.config['OAISERVER_RESUMPTION_TOKEN_EXPIRE_TIME'] = 600 + with patch.object(current_search_client, 'search', + wraps=current_search_client.search) as mock_search: + get_records() + assert mock_search.call_args[1]['scroll'] == '600s'