-
Notifications
You must be signed in to change notification settings - Fork 53
fix: minor robustness fixes on scan-txlog scripts
#3260
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
fix: minor robustness fixes on scan-txlog scripts
#3260
Conversation
|
@jnicoulaud-ledger please let us know if/once you'd like us to trigger a full CI run! You also might need to force push to rewrite your git log with |
| f"{self.url}/api/scan/v0/updates", json=payload | ||
|
|
||
| json = await self.__post_with_retry_on_statuses( | ||
| f"{self._get_current_url()}/api/scan/v0/updates", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
probably best to switch to /v2/updates as a prudent engineering measure:
splice/apps/scan/src/main/openapi/scan.yaml
Lines 374 to 384 in 6dab159
/v1/updates/{update_id}: get: deprecated: true tags: [ deprecated ] x-jvm-package: scan operationId: "getUpdateByIdV1" description: | Returns the update with the given update_id. Unlike /v0/updates/{update_id}, this endpoint returns responses that are consistent across different scan instances. Event ids returned by this endpoint are not comparable to event ids returned by /v0/updates. The order of items in events_by_id is not defined. splice/apps/scan/src/main/openapi/scan.yaml
Lines 270 to 285 in 6dab159
/v2/updates: post: tags: [external, scan] x-jvm-package: scan operationId: "getUpdateHistoryV2" description: | Returns the update history in ascending order, paged, from ledger begin or optionally starting after a record time. Compared to `/v1/updates`, the `/v2/updates` removes the `offset` field in responses, which was hardcoded to 1 in `/v1/updates` for compatibility, and is now removed. `/v2/updates` sorts events lexicographically in `events_by_id` by `ID` for convenience, which should not be confused with the order of events in the transaction, for this you should rely on the order of `root_event_ids` and `child_event_ids`. Updates are ordered lexicographically by `(migration id, record time)`. For a given migration id, each update has a unique record time. The record time ranges of different migrations may overlap, i.e., it is not guaranteed that the maximum record time of one migration is smaller than the minimum record time of the next migration, and there may be two updates with the same record time but different migration ids.
|
Maybe we could increase the default |
|
I've been thinking about improving performance by issuing parallel calls to the Scan API, but I’m not sure that’s possible. From what I can tell, pagination in |
I believe you could organize the parallel fetching by time intervals. For example, run ingestion for every day separately. The ingestion can start at the beginning of the day, and then fetch all the pages until it sees the first update after the current day. |
logdirectory if missingunclaimed_sv_rewards.pyPull Request Checklist
Cluster Testing
/cluster_teston this PR to request it, and ping someone with access to the DA-internal system to approve it./hdm_teston this PR to request it, and ping someone with access to the DA-internal system to approve it.PR Guidelines
Fixes #n, and mention issues worked on using#nMerge Guidelines