Skip to content
This repository was archived by the owner on Sep 30, 2024. It is now read-only.

repos: check for corruption during backgroud repo update#46014

Merged
burmudar merged 77 commits into
mainfrom
wb/repo/update-loop-corruption-check
Jan 9, 2023
Merged

repos: check for corruption during backgroud repo update#46014
burmudar merged 77 commits into
mainfrom
wb/repo/update-loop-corruption-check

Conversation

@burmudar

@burmudar burmudar commented Dec 30, 2022

Copy link
Copy Markdown
Contributor

During background repo update, git server can encounter errors which
indicate that the repo is corrupt but it never gets marked as corrupt,
leading to it never being cleanup as a corrupt repo would.

This change lets GitSyncer upon Error return a GitCommandError, which
allows us to inspect the stderr and determine whether the repo is
corrupt.

Test plan

Unit tests + CI

@cla-bot cla-bot Bot added the cla-signed label Dec 30, 2022
This was referenced Dec 30, 2022
Merged
Merged
Merged
@burmudar Graphite App

burmudar commented Dec 30, 2022

Copy link
Copy Markdown
Contributor Author

Current dependencies on/for this PR:

This comment was auto-generated by Graphite.

@sourcegraph-bot

sourcegraph-bot commented Dec 30, 2022 •
edited
Loading

Copy link
Copy Markdown
Contributor

Codenotify: Notifying subscribers in CODENOTIFY files for diff 198a4fb...bcbd8b1.

Notify File(s)
@indradhanush cmd/gitserver/server/server.go
cmd/gitserver/server/vcs_syncer_git.go
@ryanslade cmd/gitserver/server/server.go
cmd/gitserver/server/vcs_syncer_git.go
@sashaostrikov cmd/gitserver/server/server.go
cmd/gitserver/server/vcs_syncer_git.go

@burmudar burmudar requested a review from pjlast December 30, 2022 11:38
mrnugget
mrnugget reviewed Jan 2, 2023
View reviewed changes
Comment thread cmd/gitserver/server/vcs_syncer_git.go Outdated
Comment thread cmd/gitserver/server/vcs_syncer_git.go Outdated
@burmudar burmudar force-pushed the wb/repo/update-loop-corruption-check branch 2 times, most recently from 28c3930 to 6353553 Compare January 3, 2023 20:43
@burmudar burmudar force-pushed the wb/repo/corruption branch from ae992b3 to 7d84cab Compare January 3, 2023 21:21
@burmudar burmudar force-pushed the wb/repo/update-loop-corruption-check branch 4 times, most recently from 159a47c to edd03f4 Compare January 3, 2023 22:07
@burmudar burmudar requested a review from mrnugget January 4, 2023 07:51
@burmudar burmudar force-pushed the wb/repo/update-loop-corruption-check branch from acf857c to b344086 Compare January 4, 2023 12:09
mrnugget
mrnugget approved these changes Jan 4, 2023
View reviewed changes
Comment thread cmd/gitserver/server/server.go Outdated
@burmudar burmudar force-pushed the wb/repo/update-loop-corruption-check branch from b344086 to fbcdd3b Compare January 4, 2023 17:17
sashaostrikov
sashaostrikov approved these changes Jan 5, 2023
View reviewed changes

@sashaostrikov sashaostrikov left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

:shipit:

Comment thread cmd/gitserver/server/vcs_syncer_git.go Outdated
@burmudar burmudar force-pushed the wb/repo/update-loop-corruption-check branch from fbcdd3b to eeaf8a1 Compare January 9, 2023 10:59
burmudar added 16 commits January 9, 2023 15:08
@burmudar
fix sql query and remove field for now
7483c75
@burmudar
review comments
9b1658b
@burmudar
review feedback
0106d93 
- check for err
- remove reason
- set default timestamp to 0 value
@burmudar
reset stiched migration graph
d910b46
@burmudar
add corruption_lod column and rename migration
842fbac
@burmudar
go gen
75c24ff
@burmudar
add logCorruption method
cada296 
- log repo corruption into a JSON array
- limit repo corruption log to 10 elements
- db tests
@burmudar
fix wording and remove test
9f14ff7
@burmudar
Fix more indentation 🤦🏻
62c59d5
@burmudar
review feedback: update test
f13ed6e
@burmudar
run sg generate again
48ee040
@burmudar
test fixes
c384e3f 
- warn instead of return an error during cleanup
- delete repos so that the name can be reused
@burmudar
fix bug
f84882f 
- repo was corrupt but did not set the reason so the repo never got
  cleaned up
@burmudar
review feedback
383a7b3 
- sql query indentation
@burmudar
review feedback
be1bc85 
- use assert module
@burmudar
do not clear corruption status on err
885fc19
@burmudar burmudar force-pushed the wb/repo/corruption branch from f96852b to 885fc19 Compare January 9, 2023 13:12
@burmudar burmudar force-pushed the wb/repo/update-loop-corruption-check branch from eeaf8a1 to 38c83ac Compare January 9, 2023 13:12
@burmudar burmudar force-pushed the wb/repo/update-loop-corruption-check branch from 38c83ac to bcbd8b1 Compare January 9, 2023 14:45
@burmudar Graphite App

burmudar commented Jan 9, 2023 •
edited
Loading

Copy link
Copy Markdown
Contributor Author

⏳ This pull request is set to merge as part of a Graphite merge job
Stack job ID: hSUUYP6wzZj7kdyI1hms.
See details on graphite.dev

Base automatically changed from wb/repo/corruption to main January 9, 2023 15:24
@burmudar burmudar merged commit 19bb235 into main Jan 9, 2023
@burmudar burmudar deleted the wb/repo/update-loop-corruption-check branch January 9, 2023 15:27
@burmudar burmudar self-assigned this Feb 15, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Reviewers

@ryanslade ryanslade Awaiting requested review from ryanslade

@pjlast pjlast Awaiting requested review from pjlast

2 more reviewers

@mrnugget mrnugget mrnugget approved these changes

@sashaostrikov sashaostrikov sashaostrikov approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

@burmudar burmudar

Labels

cla-signed

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@burmudar @sourcegraph-bot @mrnugget @sashaostrikov