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

Subtract a delta of 2 minutes from cache file mtime during comparison with project mtime #29

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

maksymhopei
Copy link

Since version 8.7.1 Gitlab updates project.last_activity_at not more often than once in a minute.
This leads to the problem, when project.last_activity_at < last commit datetime, and therefore to not updating the packages.json.
@see: https://gitlab.com/gitlab-org/gitlab-ce/issues/22213

@lemoinem
Copy link
Contributor

lemoinem commented Sep 5, 2017

Thanks for your contribution. This sounds like a reasonable fix for 8.7.1 to 8.12.
However, starting with 8.12, project.last_activity_at is only updated only once per hour (see https://gitlab.com/gitlab-org/gitlab-ce/merge_requests/6391 and #21).

In this context, adding a delay of 1h+ seems too heavy and would make caching useless on any infra with a light-to-moderate level of activity, so this fix couldn't be adapted to 8.12+.

However, I'm open to give the freedom to users. Would you mind slightly modifying your PR so the delay can be configured and add the explanation and use cases in the documentation? (README)

Until there is a better alternative (although I don't know what a good alternative could be...), it sounds like an acceptable workaround to me.

@maksymhopei
Copy link
Author

Oh, that's bad... Great that you found this out.
Yes, for sure I'll put his to a configuration. Also will try to find a better approach if possible. Because hour is not really a good option for us.

@lemoinem
Copy link
Contributor

lemoinem commented Sep 6, 2017

Thanks, ping me when you push your modifications so I can see them right away.

FYI, in #21 (comment), SautdeChat suggested using the System Hook API to register a hook that could be used to invalidate the cache. Which sounds like a great solution.

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