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

Remove repository index #29

Merged
merged 3 commits into from
Nov 8, 2021
Merged

Remove repository index #29

merged 3 commits into from
Nov 8, 2021

Conversation

peterwaller-arm
Copy link
Contributor

@peterwaller-arm peterwaller-arm commented Nov 5, 2021

Draft.

We plan to remove the RepositoryIndex for now and make it fast enough to
consult the packs directly.

This removes the notion of update-index from the UI, and gives us a single source of truth: the pack indices themselves.

Depends: #27, #28 (only look at last commit).

@peterwaller-arm peterwaller-arm force-pushed the remove-on-disk-index branch 3 times, most recently from 1660e8c to 2c8d2f6 Compare November 6, 2021 23:13
@peterwaller-arm peterwaller-arm changed the title Remove on disk index Remove repository index Nov 6, 2021
@peterwaller-arm peterwaller-arm mentioned this pull request Nov 8, 2021
1 task
We plan to remove the RepositoryIndex for now and make it fast enough to
consult the packs directly.

Signed-off-by: Peter Waller <peter.waller@arm.com>
Copy link
Member

@veselink1 veselink1 left a comment

Choose a reason for hiding this comment

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

Looks good.
I'm thinking we might want to cache the indexes at some point? We will end up reading the same index several times otherwise.

src/repo/error.rs Outdated Show resolved Hide resolved
src/repo/repository.rs Show resolved Hide resolved
@peterwaller-arm
Copy link
Contributor Author

I'm thinking we might want to cache the indexes at some point? We will end up reading the same index several times otherwise.

Do you mean cache them in memory? If so, is there a specific operation you had in mind? I was thinking that if some operation needs repeated access to an index it should just retain a loaded PackIndex.

@peterwaller-arm peterwaller-arm merged commit 1f8ba71 into main Nov 8, 2021
@peterwaller-arm peterwaller-arm deleted the remove-on-disk-index branch November 8, 2021 19:01
@veselink1
Copy link
Member

Cache in memory, yes. Resolving a snapshot name opens and reads all indexes. Opening the pack to extract opens and reads the pack index again.
If we allow repacking at a later stage, and we will probably allow the user to specify multiple snapshots to pack, each one will need to be resolved separately.

@peterwaller-arm
Copy link
Contributor Author

Resolving a snapshot name opens and reads all indexes

At the moment yes, but in the future this will just involve consulting the list of snapshots at the front of the pack. I'm expecting this to be efficient enough to be able to do without the need for a cache.

If we allow repacking at a later stage, and we will probably allow the user to specify multiple snapshots to pack, each one will need to be resolved separately.

I think in this case what we coud do is first resolve the total set of packs requested, then consider each pack once, resolving requested snapshots as we go (in the order they appear in their indices). Then reorder those resolved snapshots back to what the user requested.

@veselink1
Copy link
Member

I forgot that we're planning to pull the snapshot tags at the front. We'll have to see how that plays out in practice I guess.

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.

None yet

2 participants