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

op-supervisor: Add log db #10902

Merged
merged 26 commits into from
Jun 24, 2024
Merged

op-supervisor: Add log db #10902

merged 26 commits into from
Jun 24, 2024

Conversation

ajsutton
Copy link
Contributor

@ajsutton ajsutton commented Jun 18, 2024

Description

Introduces an append only database to record logs for a single chain. Reorgs can be handled by rewinding the chain back to a specific block then adding the new canonical logs.

This implements the basic log storage and search functionality but has two key pieces not yet implemented:

  1. Storing and returning executing message dependencies for logs
  2. Recovering the database after write errors, particularly when a partial write occurs. An error is returned to the caller to indicate the write failed, but there's no code to handle truncating any partially written data or ensuring the data cached in memory is rolled back.

These items will be added in a follow up PR.

Also note that this doesn't attempt to track different levels of safety. Ideally an outer wrapper can track that based on block number. Storing those head levels to disk may require them being handled inside this struct though ideally I'd like them to just be a prefix and manage to keep this struct completely focussed on storing the log data.

A simple RWMutex is used to provide thread safety, it may be better to remove that again in the future and have thread safety be handled by the wrapper. Or if we want to maximise performance it may be possible to have more fine-grained locking so that checking if logs exist can be safely done in parallel with adding blocks, with only rewinds needing exclusive access. Going to avoid adding that complexity until we know it's needed.

Tests

Added unit tests. Each test is run first by writing and reading from the same DB instance, then writes and performs the reads from a separate instance to confirm it can be reread purely from the information on disk. Also implemented a set of invariant checks that are run after each test to verify key properties of the database format.

Metadata

Copy link
Contributor

semgrep-app bot commented Jun 20, 2024

Semgrep found 8 todos_require_linear findings:

Please create a GitHub ticket for this TODO.

Ignore this finding from todos_require_linear.

@ajsutton ajsutton marked this pull request as ready for review June 21, 2024 00:58
Copy link
Contributor

@protolambda protolambda left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Functionality is good, thank you! I have some suggestions to introduce more fine typing and separate the file-read/write layer from the entry-read/write layer, to make things more readable.

op-supervisor/supervisor/backend/db/db.go Outdated Show resolved Hide resolved
op-supervisor/supervisor/backend/db/db.go Outdated Show resolved Hide resolved
op-supervisor/supervisor/backend/db/db.go Outdated Show resolved Hide resolved
op-supervisor/supervisor/backend/db/db.go Outdated Show resolved Hide resolved
op-supervisor/supervisor/backend/db/db.go Outdated Show resolved Hide resolved
op-supervisor/supervisor/backend/db/db.go Show resolved Hide resolved
op-supervisor/supervisor/backend/db/db.go Outdated Show resolved Hide resolved
@ajsutton ajsutton changed the title op-supervisor: Add simple log db op-supervisor: Add log db Jun 21, 2024
@ajsutton ajsutton requested a review from protolambda June 21, 2024 06:10
Copy link
Contributor

@Inphi Inphi left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

just some quick notes. I'll take a closer look later.

Copy link
Contributor

@protolambda protolambda left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Very nice!

@protolambda protolambda added this pull request to the merge queue Jun 24, 2024
Merged via the queue into develop with commit 1cf5239 Jun 24, 2024
58 checks passed
@protolambda protolambda deleted the aj/log-db branch June 24, 2024 21:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants