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

proposal: async update pull_time and pull_count #183

Conversation

chlins
Copy link
Member

@chlins chlins commented Dec 3, 2021

Fix: goharbor/harbor#15963

Signed-off-by: chlins chenyuzh@vmware.com

@chlins chlins force-pushed the proposal/async-update-pull_time-and-pull_count branch from e4ecd85 to d6cb436 Compare December 3, 2021 09:24
wy65701436
wy65701436 previously approved these changes Dec 8, 2021
Copy link
Contributor

@wy65701436 wy65701436 left a comment

Choose a reason for hiding this comment

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

lgtm

Signed-off-by: chlins <chenyuzh@vmware.com>
@chlins chlins force-pushed the proposal/async-update-pull_time-and-pull_count branch from d6cb436 to 487f057 Compare December 8, 2021 08:37
@tianon
Copy link
Member

tianon commented Dec 15, 2021

This is slow specifically because it's updating an existing database record, right?

What if instead the code just did an append/insert to some "audit" type table for each event (which should be very fast, given the way the database journaling works) and then have a job that periodically (on a delayed trigger of some kind, maybe?) tallied those up for a combined update that does multiple pulls worth at once?

@chlins
Copy link
Member Author

chlins commented Dec 16, 2021

This is slow specifically because it's updating an existing database record, right?

What if instead the code just did an append/insert to some "audit" type table for each event (which should be very fast, given the way the database journaling works) and then have a job that periodically (on a delayed trigger of some kind, maybe?) tallied those up for a combined update that does multiple pulls worth at once?

@tianon Thanks your advice, but actually we already have the access logs in the audit_log table now which like you say, but the table is too heavy, there are many issues about the audit log is too big or too slow, we also consider some ways to rotate this table, so from this table to retrieve access log and combine them maybe also bring up other performance issues.

Copy link
Contributor

@wy65701436 wy65701436 left a comment

Choose a reason for hiding this comment

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

lgtm

@chlins chlins merged commit 50c5abb into goharbor:master Jan 5, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

DB lock under 1k concurrency pull
6 participants