Skip to content
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

LogPoller support for MaxLogsKept #1338

Closed
wants to merge 55 commits into from

Conversation

reductionista
Copy link
Collaborator

@reductionista reductionista commented Aug 21, 2024

Motivation

Presently, LogPoller supports only time-based retention, via the Retention field in the filters passed to RegisterFilter. The MaxLogsKept field was added earlier in anticipation of the need for also supporting recency-count based retention. One example of a case where time based retention is risky is the Transmit event in the OCR Contract Transmitter. No matter how long the retention period is set to, there's a chance the node will be down for longer than that, and miss logs when it comes back up. This would make a bad situation even worse, because the transmit event would never be picked up at all.

Solution

This implements the MaxLogsKept feature in LogPoller. When specified, this field tells LogPoller it's okay to prune logs matching a filter if there are at least MaxLogsKept more recent matching logs in the db.
In the example above, this avoid storing any more logs than needed while always having the latest transmit event available. In this case, older transmit events are no longer relevant if there is a more recent one.
In general, this should be just as useful for anything accessed only via ChainReader's GetLatestValue() method rather than QueryKey().

A log may be pruned either because it's too old in terms of time or in terms of the number of logs being saved. It need not satisfy both theRetention and MaxLogsKeptcriteria in order to get pruned.

@reductionista reductionista changed the title Max logs kept tests Max logs kept tests: DO NOT MERGE Aug 21, 2024
@reductionista reductionista changed the title Max logs kept tests: DO NOT MERGE Testing MaxLogsKept feature: DO NOT MERGE Aug 21, 2024
@reductionista reductionista changed the title Testing MaxLogsKept feature: DO NOT MERGE Alpha testing of MaxLogsKept LogPoller feature: DO NOT MERGE Aug 21, 2024
@reductionista reductionista changed the title Alpha testing of MaxLogsKept LogPoller feature: DO NOT MERGE Testing of MaxLogsKept LogPoller feature: DO NOT MERGE Aug 21, 2024
@reductionista reductionista changed the title Testing of MaxLogsKept LogPoller feature: DO NOT MERGE LogPoller support for MaxLogsKept Aug 21, 2024
@cl-sonarqube-production
Copy link

Quality Gate failed Quality Gate failed

Failed conditions
35.1% Coverage on New Code (required ≥ 75%)

See analysis details on SonarQube

Copy link
Contributor

This PR is stale because it has been open 45 days with no activity. Remove stale label or comment or this will be closed in 10 days.

@github-actions github-actions bot added the Stale label Nov 27, 2024
Copy link
Contributor

github-actions bot commented Dec 8, 2024

This PR was closed because it has been stalled for 10 days with no activity.

@github-actions github-actions bot closed this Dec 8, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant