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

Drop DB v6 indexes on close #2335

Merged
merged 2 commits into from
Dec 16, 2024
Merged

Drop DB v6 indexes on close #2335

merged 2 commits into from
Dec 16, 2024

Conversation

wagoodman
Copy link
Contributor

@wagoodman wagoodman commented Dec 16, 2024

This PR makes the following adjustments to the v6 store:

When opening a DB for reading or writing:

  • use 16KB page size (tended to reduce the overall size of the DB for distribution)

When opening an EMPTY db for writing:

  • cache size increased to 1GB
  • mmap limit increased to 1GB

When closing a newly-populated DB:

  • drop all indexes. This helps to keep the distribution size way down. Testing shows that re-building these indexes is quite cheap (~2 seconds, milage may vary)

When importing a DB:

  • automigrate is ran, re-creating any indexes that were dropped before distribution (v6.Hydrater)

Other changes made in this PR:

  • moves internal.NewDB to v6.NewLowLevelDB so that it has direct access to initial data and models to be migrated (so getting a gorm.DB instance is consistent)
  • allowed TUI in import DB

@wagoodman wagoodman added the changelog-ignore Don't include this issue in the release changelog label Dec 16, 2024
Signed-off-by: Alex Goodman <[email protected]>
@wagoodman wagoodman marked this pull request as ready for review December 16, 2024 21:54
@wagoodman wagoodman self-assigned this Dec 16, 2024
@wagoodman wagoodman enabled auto-merge (squash) December 16, 2024 22:18
grype/db/internal/gormadapter/open.go Outdated Show resolved Hide resolved
"immutable=1",
"cache=shared",
"mode=ro",
var heavyWriteStatements = []string{
Copy link
Contributor

Choose a reason for hiding this comment

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

nit: this doesn't seem to have options directly related to write performance but rather memory, with an option named allowLargeMemoryFootprint should this be largeMemoryFootprintStatements?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

#2335 (comment) so when there are heavy writes expected we want to allow for a larger memory footprint (to improve write performance).

}
}

if cfg.truncate && cfg.allowLargeMemoryFootprint {
Copy link
Contributor

Choose a reason for hiding this comment

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

Is the memory footprint related to truncating?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

it is -- when we start with an empty DB we will be doing a lot of writing (and allowing for a larger memory footprint in those cases allows for writing to the DB much faster).

grype/db/v4/store/store.go Show resolved Hide resolved
cmd/grype/cli/commands/db_import.go Outdated Show resolved Hide resolved
grype/db/v6/installation/curator_test.go Outdated Show resolved Hide resolved
@wagoodman wagoodman disabled auto-merge December 16, 2024 22:25
Copy link
Contributor

@kzantow kzantow left a comment

Choose a reason for hiding this comment

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

(None of the prior review was blocking; didn't approve then since it was auto-merge)

Signed-off-by: Alex Goodman <[email protected]>
@wagoodman wagoodman enabled auto-merge (squash) December 16, 2024 22:44
@wagoodman wagoodman merged commit 69330e5 into main Dec 16, 2024
10 checks passed
@wagoodman wagoodman deleted the v6-drop-index branch December 16, 2024 23:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
changelog-ignore Don't include this issue in the release changelog
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

2 participants