Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

perf: api: add indexes to event topics and emitter addr #11477

Merged
merged 2 commits into from
Jan 31, 2024

Conversation

i-norden
Copy link
Contributor

@i-norden i-norden commented Dec 4, 2023

Related Issues

Proposed Changes

Add indexes to the events.db event_entry.key column and event.emitter_addr column.

Additional Info

These indexes significantly improve the performance of eth_getLog queries with filters that contain multiple contract addresses or topics to search against, which is often the case for getLog consumers coming from the ethereum domain. In our tests a query filter with 10 topics saw a >100x improvement in response time (from ~45s to ~0.4s). Applying these indexes increased the size of our archive node events.db from 383 MB to 428 MB.

Checklist

Before you mark the PR ready for review, please make sure that:

  • Commits have a clear commit message.
  • PR title is in the form of of <PR type>: <area>: <change being made>
    • example: fix: mempool: Introduce a cache for valid signatures
    • PR type: fix, feat, build, chore, ci, docs, perf, refactor, revert, style, test
    • area, e.g. api, chain, state, market, mempool, multisig, networking, paych, proving, sealing, wallet, deps
  • If the PR affects users (e.g., new feature, bug fix, system requirements change), update the CHANGELOG.md and add details to the UNRELEASED section. (N/A)
  • New features have usage guidelines and / or documentation updates in (N/A)
  • Tests exist for new functionality or change in behavior (N/A)
  • CI is green (I believe the remaining red are some CI finickyness)

@i-norden i-norden requested a review from a team as a code owner December 4, 2023 17:56
Copy link
Contributor

@fridrik01 fridrik01 left a comment

Choose a reason for hiding this comment

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

LGTM

@Stebalien Stebalien merged commit 95fb198 into filecoin-project:master Jan 31, 2024
@rvagg
Copy link
Member

rvagg commented Apr 24, 2024

Given that there have been a few reports of people running master and then a v1.26.x branch and failing to start lotus because of this PR, here's a couple of thoughts for fixes:

  1. Nuke your events database

rm sqlite/events.db*

The downside of this is that you lose history. Though you could back-fill; it just might take time.

  1. Downgrade the events database so it's strictly back to the v1.26.x form

This is probably the cleanest form, although I haven't tried this but it should work. Stop lotus and with sqlite3 installed on your CLI:

sqlite3 sqlite/events.db <<-EOD

BEGIN TRANSACTION;

DROP INDEX IF EXISTS event_emitter_addr;
DROP INDEX IF EXISTS event_entry_key_index;
DELETE FROM _meta WHERE version=3;

COMMIT;

EOD
  1. Downgrade just the version of the database

Similar to option 2, but a little more light-touch, run sqlite sqlite/events.db and execute UPDATE _meta SET version=2 WHERE version=3. This will leave the 2 indexes in place, which should be fine except there's a small possibility it may conflict with future changes to the database. You'll just have to be attuned to that risk when you upgrade a future lotus and see an error about this (error messages have been improved on master for these situations so it should be a little more obvious that you have schema conflicts).

@aarshkshah1992
Copy link
Contributor

This wont work because of

// metadata containing version of schema
	`CREATE TABLE IF NOT EXISTS _meta (
		version UINT64 NOT NULL UNIQUE
	)`,

`INSERT OR IGNORE INTO _meta (version) VALUES (1)`,
	`INSERT OR IGNORE INTO _meta (version) VALUES (2)`,
	`INSERT OR IGNORE INTO _meta (version) VALUES (3)`,

So when we move the row from version 3 to version 2, it violates the uniqueness constraint.
We'll just drop the row where version=3

@rvagg
Copy link
Member

rvagg commented Apr 24, 2024

good catch, updated comment and replaced UPDATE _meta SET version=2 WHERE version=3; with DELETE FROM _meta WHERE version=3;

@i-norden
Copy link
Contributor Author

Is there more info on how this is breaking nodes? We were able upgrade to and run with this on a full archive node.

@aarshkshah1992
Copy link
Contributor

Hey @i-norden

This will only break for nodes migrating from master to the current release.
It's because master has an upgraded version of the events DB schema that the current release still hasn't upgaded to.
So when you move from master -> current release, it complains that it dosen't expect the upgraded events DB schema that master earlier migrated to. However, it's pretty trivial to fix by using the SQL statements @rvagg posted above.

@i-norden
Copy link
Contributor Author

Ah got it, thanks! Another option would be to cherry-pick this on-top of the release, but there will be issues again if the version 3 that makes it into a release does not match this one. Could put any new changes to the events db schema into a version 4, but having affected parties drop the version 3 meta keeps things cleaner for everybody else.

Agree that it is unlikely things will conflict with the lingering indexes. Reapplication of this version 3 won't throw since it uses IF NOT EXISTS and I can't think of any migration that conflicts on an index other than trying to create one that already exists or delete one that doesn't.

# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants