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

batch api for eviction #3554

Merged

Conversation

problame
Copy link
Contributor

@problame problame commented Feb 7, 2023

ref #3552 (comment)

What do you think, @koivunej ?

Comment on lines +957 to 958
self.delete_historic_layer(layer_removal_cs, Arc::clone(local_layer), &mut updates)?;
updates.insert_historic(new_remote_layer);
Copy link
Member

Choose a reason for hiding this comment

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

If you rebase, you should see the replace now being used here in a conflict.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah, right now this branch targets #3552 but I think I'll spin it out once your #3513 is merged.

Ok(results)
}

async fn evict_layer_batch_impl(
Copy link
Member

Choose a reason for hiding this comment

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

This is no longer async right?

IMHO the layers could be kept locked as well, since the other lock is already. Also, only one batch should happen. Or perhaps there's a reason why multiple batches?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is no longer async right?

Right. Will remove it for now and keep using the synchronous unlink code.

IMHO the layers could be kept locked as well, since the other lock is already.Also, only one batch should happen. Or perhaps there's a reason why multiple batches?

Yeah. Both were oversights from my side, sorry about that.

@problame problame merged commit 5a9fa32 into problame/automatic-eviction Feb 7, 2023
@problame problame deleted the problame/for/automatic-eviction branch February 7, 2023 13:00
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.

2 participants