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'