Skip to content

Indexes 7: Adds a lock file around index generation and updates#1354

Merged
dcookspi merged 1 commit into
mainfrom
index-7-lockfile-for-indexgen
May 14, 2026
Merged

Indexes 7: Adds a lock file around index generation and updates#1354
dcookspi merged 1 commit into
mainfrom
index-7-lockfile-for-indexgen

Conversation

@dcookspi
Copy link
Copy Markdown
Collaborator

@dcookspi dcookspi commented Apr 16, 2026

This adds a lock file around index generation and update processing. The lock for the index file is created, via a FileLock object, before the underlying repo is read and kept until the index has been generated and written out. The same duration of locking is done for updating the index.

A FileLock has configurable settings for the maximum number of tries to get the lock before giving up, and the amound of time to sleep between tries, as well as the kind of file being locked (used in log messages). It also contains sentry integration when the sentry feature is enabled.

This is the 7th of the chained PRs for adding indexes to spk solves:

  1. Indexes 1: Change Package and related traits to not return references to fields #1336
  2. Indexes 2: Add new_unchecked() constructors to spk schema objects #1337
  3. Indexes 3: Adds flatbuffers schema and SolverPackageSpec for indexes to spk #1338
  4. Indexes 4: Adds Indexes for SPK repositories #1339
  5. Indexes 5: Adds spk repo index subcommand for index generation and updates #1340
  6. Indexes 6: Changes version_filter field in index schema #1344
  7. this PR
  8. Indexes 8: Fixes the spk build or mkb crash with indexes enabled #1355
  9. Indexes 9: Adds messaging on package events to kafka #1356
  10. Indexes 10: Adds index update listeners and Indexer #1363

@dcookspi dcookspi self-assigned this Apr 16, 2026
@dcookspi dcookspi added enhancement New feature or request SPI AOI Area of interest for SPI pr-chain This PR doesn't target the main branch, don't merge! labels Apr 16, 2026
@dcookspi dcookspi force-pushed the index-7-lockfile-for-indexgen branch from a0324cf to 85050e7 Compare April 16, 2026 00:58
@dcookspi dcookspi requested review from jrray and rydrman April 16, 2026 01:09
@codecov
Copy link
Copy Markdown

codecov Bot commented Apr 16, 2026

Codecov Report

❌ Patch coverage is 2.97030% with 196 lines in your changes missing coverage. Please review.

Files with missing lines Patch % Lines
crates/spk-storage/src/storage/flatbuffer_index.rs 0.00% 175 Missing ⚠️
crates/spk-cli/cmd-repo/src/cmd_repo.rs 0.00% 11 Missing ⚠️
crates/spk/src/cli.rs 0.00% 10 Missing ⚠️

📢 Thoughts on this report? Let us know!

Comment thread crates/spk-cli/cmd-repo/src/cmd_repo.rs Outdated
Comment thread crates/spk-storage/src/storage/flatbuffer_index.rs Outdated
Comment thread crates/spk-storage/src/storage/flatbuffer_index.rs Outdated
Comment thread crates/spk-storage/src/storage/flatbuffer_index.rs Outdated
Comment thread crates/spk-storage/src/storage/flatbuffer_index.rs
Comment thread crates/spk-storage/src/storage/flatbuffer_index.rs Outdated
Comment thread crates/spk-storage/src/storage/flatbuffer_index.rs Outdated
Comment on lines +153 to +156
// Need to keep these if-statements separate
// to allow for two different error handling cases.
#[allow(clippy::collapsible_if)]
if let Some(err) = result.err() {
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Suggested change
// Need to keep these if-statements separate
// to allow for two different error handling cases.
#[allow(clippy::collapsible_if)]
if let Some(err) = result.err() {
if let Some(err) = result.err() {

This can be removed, it was the unnecessary outer if causing the lint.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

I've removed that now.


/// Unlock this file lock by removing the lock file
///
/// # Safety:
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Suggested change
/// # Safety:
/// # Safety

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

I've updated this.

@jrray
Copy link
Copy Markdown
Collaborator

jrray commented May 11, 2026

Is it intentional that this PR has index-5 as the merge target and not index-6?

@dcookspi
Copy link
Copy Markdown
Collaborator Author

Is it intentional that this PR has index-5 as the merge target and not index-6?

It has index-5-zplus-1-... as the target, which is really number 6, but it wasn't renamed before its PR was made.

@dcookspi dcookspi requested a review from jrray May 12, 2026 22:09
@dcookspi dcookspi force-pushed the index-5-zplus-1-versionfilters-not-strings branch from 424f49a to 3b3ed45 Compare May 14, 2026 21:43
Base automatically changed from index-5-zplus-1-versionfilters-not-strings to main May 14, 2026 21:59
Signed-off-by: David Gilligan-Cook <dcook@imageworks.com>
@dcookspi dcookspi force-pushed the index-7-lockfile-for-indexgen branch from 501db10 to d782c1b Compare May 14, 2026 22:01
@dcookspi dcookspi merged commit 157d2f7 into main May 14, 2026
10 checks passed
@dcookspi dcookspi deleted the index-7-lockfile-for-indexgen branch May 14, 2026 22:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement New feature or request pr-chain This PR doesn't target the main branch, don't merge! SPI AOI Area of interest for SPI

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants