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

Fix sync never ending when pantry is not updated in 24 hours #73

Closed
wants to merge 1 commit into from
Closed

Fix sync never ending when pantry is not updated in 24 hours #73

wants to merge 1 commit into from

Conversation

felipecrs
Copy link
Contributor

@felipecrs felipecrs commented Jul 6, 2024

To be clear, pkgx is completely broken by now because of this issue.

It turns out that the function that detects whether the sync was last synced recently isn't working properly, because it's relying on the mtime of the pantry/projects folder, which is set to its original mtime when downloaded by either git or tar.

This means the function would always detect outdated sync if there was no commit in the pantry repo in the last 24 hours.

This was just a quick way around it. Other solutions I tried:

  1. ctime (change time). This would work better, but Deno doesn't support it. (node does)

  2. tar's --touch to reset modification time, but it only works for files not for directories. Also, the pantry could have been downloaded with git.

PS: tests are failing but the code is working, and I'm using it in my fork. I will not bother fixing tests as apparently no one is merging code here anyway.

@jhheider
Copy link
Contributor

jhheider commented Jul 8, 2024

@felipecrs that's a good find. i know main is currently a work-in-progress, and max's attention has been diverted for the moment. in the interim, i can schedule the ci.pantry.tgz.yml job to run at midnight, which should hopefully alleviate this until we decide on a proper solution.

mxcl added a commit that referenced this pull request Sep 5, 2024
We shouldn't need this since we sync for various pkg-not-found situations.

Closes #73
Closes pkgxdev/pkgx#1021
@mxcl
Copy link
Member

mxcl commented Sep 5, 2024

Tried to find how this would loop infinitely but couldn't see it so just decided to remove neglected altogether in #75 since it is dumb.

We force sync for all situations where a pkg might be newer than what we have locally already so this neglected check was overkill and dumb anyway.

mxcl added a commit that referenced this pull request Sep 5, 2024
We shouldn't need this since we sync for various pkg-not-found situations.

Closes #73
Closes pkgxdev/pkgx#1021
mxcl added a commit that referenced this pull request Sep 5, 2024
We shouldn't need this since we sync for various pkg-not-found situations.

Closes #73
Closes pkgxdev/pkgx#1021
@felipecrs
Copy link
Contributor Author

Tried to find how this would loop infinitely

Try to patch this line:

- return (Date.now() - stat.mtime.getTime()) > 24 * 60 * 60 * 1000
+ return (Date.now() - stat.mtime.getTime()) > 15 * 60 * 1000

And you should be able to reproduce it. As long as this time is longer than the time the pantry was updated for the last time, you should be able to reproduce it:

image

@jhheider
Copy link
Contributor

jhheider commented Sep 5, 2024

As long as this time is longer than the time the pantry was updated for the last time

to that end, you can probable just take out all the multiplication. The pantry should always be older than now, so it should loop immediately.

@felipecrs
Copy link
Contributor Author

Ops, btw I meant "shorter", not "longer".

@mxcl mxcl closed this in c6371cd Sep 6, 2024
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.

3 participants