Repository navigation
Conversation
Reviewer's GuideOAI-PMHのresumption tokenおよびElasticsearch scrollのkeep-alive既定値を24時間から1時間へ短縮し、設定の上書きと初回・継続ページでのscroll伝播をテストで担保する変更です。 Sequence diagram for OAI-PMH scroll keep-alive across pagessequenceDiagram
participant Harvester
participant OAI_Server
participant Elasticsearch
Harvester->>OAI_Server: ListRecords / ListIdentifiers
OAI_Server->>Elasticsearch: search(scroll=OAISERVER_RESUMPTION_TOKEN_EXPIRE_TIME)
Elasticsearch-->>OAI_Server: First page and resumptionToken
OAI_Server-->>Harvester: First page and resumptionToken
Harvester->>OAI_Server: ListRecords / ListIdentifiers with resumptionToken
OAI_Server->>Elasticsearch: scroll(keep_alive=OAISERVER_RESUMPTION_TOKEN_EXPIRE_TIME)
Elasticsearch-->>OAI_Server: Next page and resumptionToken
OAI_Server-->>Harvester: Next page and resumptionToken
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the ⚙️ Run configuration
You can disable this status message by setting the Use the checkbox below for a quick retry:
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
🔍 Claude レビュー統合指摘はありません。 次にすること: 裏を取れた問題はなく、このまま進めて問題ありません。外部データは要約と「レビュー省略」の通知だけで、裁定すべき指摘スレッドはありませんでした。設定値は query.py:159 の1か所から読まれ、初回の search(168行目)と継続ページの scroll(250行目)の両方で モデル sonnet ／ 2 回実行して和集合 ／ コスト $0.2175。同じ入力でも結果が揺れるため複数回まわし、一部のパスでしか挙がらなかったものには回数を添えています 他レビューを踏まえた自動レビューです。誤りが含まれることがあります。 |
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Hey - I've found 1 issue
Prompt for AI Agents
Please address the comments from this code review:
## Individual Comments
### Comment 1
<location path="modules/invenio-oaiserver/invenio_oaiserver/config.py" line_range="38" />
<code_context>
"""
-OAISERVER_RESUMPTION_TOKEN_EXPIRE_TIME = 24 * 60 * 60
+OAISERVER_RESUMPTION_TOKEN_EXPIRE_TIME = 60 * 60
"""The expiration time of a resumption token in seconds.
</code_context>
<issue_to_address>
**Scrolls exceed cluster limit**
When an Elasticsearch deployment has `search.max_keep_alive` configured below 3600 seconds, `get_records` passes the configured value as the Elasticsearch scroll keep-alive on both the initial search and later scroll requests, with no cap or error handling. Elasticsearch rejects those requests, so OAI-PMH record and identifier harvesting fails.
Validate the effective keep-alive against the Elasticsearch limit at startup, or ensure the setting is kept within that limit before enabling this default.
</issue_to_address>Sourcery assessment
Approval pending. 1 finding to address first.
Blocking findings: modules/invenio-oaiserver/invenio_oaiserver/config.py:38
| """ | ||
|
|
||
| OAISERVER_RESUMPTION_TOKEN_EXPIRE_TIME = 24 * 60 * 60 | ||
| OAISERVER_RESUMPTION_TOKEN_EXPIRE_TIME = 60 * 60 |
There was a problem hiding this comment.
🟠 High · Scrolls exceed cluster limit
When an Elasticsearch deployment has search.max_keep_alive configured below 3600 seconds, get_records passes the configured value as the Elasticsearch scroll keep-alive on both the initial search and later scroll requests, with no cap or error handling. Elasticsearch rejects those requests, so OAI-PMH record and identifier harvesting fails.
Validate the effective keep-alive against the Elasticsearch limit at startup, or ensure the setting is kept within that limit before enabling this default.
Prompt for AI agents
In `modules/invenio-oaiserver/invenio_oaiserver/config.py` at line 38:
**Scrolls exceed cluster limit**
When an Elasticsearch deployment has `search.max_keep_alive` configured below 3600 seconds, `get_records` passes the configured value as the Elasticsearch scroll keep-alive on both the initial search and later scroll requests, with no cap or error handling. Elasticsearch rejects those requests, so OAI-PMH record and identifier harvesting fails.
Validate the effective keep-alive against the Elasticsearch limit at startup, or ensure the setting is kept within that limit before enabling this default.
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 <noreply@anthropic.com>
API インベントリ差分（件数のみ）
ベースラインとの差分API インベントリ差分レポート
判定: ❌ FAIL (FAIL 1 / WARN 3)サマリ
[FAIL] G2 認証系デコレータが削除された — 3件
[WARN] W2 実装本体が変化(data_op / 情報露出を再確認) — 9件
[WARN] W6 依存パッケージの版が変化した — 2件
[WARN] W5 監視対象 config が変化した — 2件
台帳との突き合わせ(生成されませんでした) ソース由来の経路検知(生成されませんでした) |
変更内容
OAISERVER_RESUMPTION_TOKEN_EXPIRE_TIMEの既定値を24 * 60 * 60(86400 秒)から60 * 60(3600 秒)に変更Default: 60 seconds)を直し、この値が scroll の keep-alive にも使われることと、ES のsearch.max_keep_aliveとの関係を追記test_app.py: 既定値が 3600 であること、環境側の設定で上書きできることtest_query.py: 1 ページ目と 2 ページ目以降の両方で、設定値が ES に scroll の keep-alive として渡ること目的
ES データノードのヒープが数日かけて増え続ける問題への対応です。
この設定値は resumptionToken の有効期限であると同時に、ListRecords / ListIdentifiers が開く ES scroll の keep-alive にも使われます(
query.py:159,168)。scroll は最終ページを返したときにしかclear_scrollされません(query.py:265-270)。そのため、1 ページ目で止まるクローラーや、途中で中断したハーベストの scroll コンテキストは、24 時間ヒープに残っていました。検証結果(ES 6.8.23)
get_records()で、1 ページ目で放棄した scroll が残ること、最後まで取り切れば閉じることを再現したclear_scrollしなくても scroll が使っていたヒープは解放される。1 本あたり約 78 KB(1 シャード、100 件、ソートあり)で、繰り返しても溜まらない影響
badResumptionTokenになります。24 時間のときより厳しくなりますが、1 ページに 1 時間以上かける実運用のハーベスタは想定しにくいですinstance.cfgなど)で値を設定している場合は、その値が優先されます(テストで確認済み)search.max_keep_aliveは既定の 24h のままで 3600 秒の要求を受け付け、期限切れの scroll は ES が自動で回収しますDELETE /_search/scroll/_allを 1 回実行してください範囲外
同じ resumptionToken で再送すると、ES の scroll が進んでしまい 1 ページ分を取りこぼす問題も見つかっています。挙動が変わる修正になるため、別の PR で扱います。
動作確認
scripts/ci/run-local.sh invenio-oaiserver: 104 passed, 3 skipped, 1 xfailed🤖 Generated with Claude Code
Summary by Sourcery
Shorten the default OAI-PMH scroll keep-alive to reduce Elasticsearch heap usage while preserving configurable harvest behavior.
Bug Fixes:
Enhancements:
Tests: