Skip to content

Fix commit-graph expiration#1647

Merged
derrickstolee merged 2 commits intomicrosoft:masterfrom
derrickstolee:commit-graph-expire-fix
Apr 2, 2020
Merged

Fix commit-graph expiration#1647
derrickstolee merged 2 commits intomicrosoft:masterfrom
derrickstolee:commit-graph-expire-fix

Conversation

@derrickstolee
Copy link
Contributor

Wow, this was really not working as expected.

See microsoft/git#255 for how broken the --expire-time argument was.

Fix this by using the fixed argument and passing a datetime instead of an offset by seconds. This will provide a longer window for old commit-graph files, but apparently we've been leaving turd files around for a long time without anyone noticing.

Signed-off-by: Derrick Stolee <dstolee@microsoft.com>
Signed-off-by: Derrick Stolee <dstolee@microsoft.com>
@derrickstolee derrickstolee force-pushed the commit-graph-expire-fix branch from 18b1f80 to af74760 Compare April 2, 2020 12:07
@derrickstolee derrickstolee requested a review from kewillford April 2, 2020 12:36
@derrickstolee derrickstolee changed the title [PR Build] Fix commit-graph expiration Fix commit-graph expiration Apr 2, 2020
@derrickstolee
Copy link
Contributor Author

/azp run PR - Windows - Build and Unit Test

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@derrickstolee
Copy link
Contributor Author

/azp run PR - Windows - Functional Tests

@derrickstolee
Copy link
Contributor Author

/azp run PR - Windows - Functional Tests (Sparse Mode)

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

1 similar comment
@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@derrickstolee
Copy link
Contributor Author

/azp run PR - Windows - Extra

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

{
if (expireTimeDateString == null)
{
expireTimeDateString = DateTime.Now.Subtract(TimeSpan.FromDays(1)).ToShortDateString();
Copy link
Member

Choose a reason for hiding this comment

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

Before we chose to expire after 1 hour and now we are after 1 day. I know that this was never working before and I don't think that will be an issue because I don't think it should be creating that many commit graphs but wanted to get your thoughts. Thanks!

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The important thing is that it doesn't expire things right away, and there are some concerns about timezones and things when it parses the expiry date. So either we get really specific about the time (including timezone offset) or we can just go "at least 24 hours ago, give or take timezone shifts of +/-12 hours".

Since we've been running with the lack of expirations for this long, having a slightly longer expire window will not be a problem.

@derrickstolee derrickstolee merged commit 49fc874 into microsoft:master Apr 2, 2020
@derrickstolee derrickstolee mentioned this pull request Apr 15, 2020
derrickstolee added a commit that referenced this pull request Apr 15, 2020
Includes the following PRs:

* #1642: Update Git to v2.26.0 
* #1647: Fix commit-graph expiration
* #1652: RepoRegistry: ignore non-existent repos
* #1653: Update Git to include v2.26.1

The update to include Git v2.26.0 comes with a new default backend for `git rebase`. By switching from the `apply` backend to the `merge` backend, some rebase situations that would previously fail will now succeed automatically. This comes with a slight performance drawback, but it is the direction that the Git community has decided to go. Please see [the Git documentation](https://git-scm.com/docs/git-rebase#_behavioral_differences) for more details.
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