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

repos: Add corrupted to repos statistics#46410

Merged
burmudar merged 6 commits into
mainfrom
wb/repo/corruption-filter-count
Jan 16, 2023
Merged

repos: Add corrupted to repos statistics#46410
burmudar merged 6 commits into
mainfrom
wb/repo/corruption-filter-count

Conversation

@burmudar

@burmudar burmudar commented Jan 13, 2023

Copy link
Copy Markdown
Contributor

Add the count of currently corrupted repos to
gitserver_repos_statistics and repo_statistics

  • Add the corrupted column to gitserver_repos_statistics and repo_statistics
  • Update the repos on_update trigger to add the corrupted value to the
    statistics tables

Test plan

unit tests

@cla-bot cla-bot Bot added the cla-signed label Jan 13, 2023
@burmudar Graphite App

burmudar commented Jan 13, 2023 •
edited
Loading

Copy link
Copy Markdown
Contributor Author

Current dependencies on/for this PR:

This comment was auto-generated by Graphite.

This was referenced Jan 13, 2023
Merged
Merged
@sourcegraph-bot

sourcegraph-bot commented Jan 13, 2023 •
edited
Loading

Copy link
Copy Markdown
Contributor

Codenotify: Notifying subscribers in CODENOTIFY files for diff b445895...07ca3d1.

Notify File(s)
@eseliger internal/database/repos_test.go
@indradhanush cmd/gitserver/server/cleanup.go
cmd/gitserver/server/server.go
@ryanslade cmd/gitserver/server/cleanup.go
cmd/gitserver/server/server.go
@sashaostrikov cmd/gitserver/server/cleanup.go
cmd/gitserver/server/server.go
@unknwon internal/database/repos_test.go

@burmudar burmudar requested a review from a team January 13, 2023 11:37
@burmudar burmudar self-assigned this Jan 13, 2023
sashaostrikov
sashaostrikov approved these changes Jan 13, 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.

LGTM!
I'm not a PLSQL expert, didn't take a very close look to that part.

Comment thread internal/database/repo_statistics_test.go Outdated
burmudar
burmudar commented Jan 13, 2023
View reviewed changes
Comment thread migrations/frontend/1673351808_add_repo_corruption_stat/up.sql Outdated
@github-actions

github-actions Bot commented Jan 13, 2023

Copy link
Copy Markdown
Contributor

❌ Problem: the label i-acknowledge-this-goes-into-the-release is absent.
👉 What to do: we're in the next Sourcegraph release code freeze period. If you are 100% sure your changes should get released or provide no risk to the release, add the label your PR with i-acknowledge-this-goes-into-the-release.

3 similar comments
@github-actions

github-actions Bot commented Jan 13, 2023

Copy link
Copy Markdown
Contributor

❌ Problem: the label i-acknowledge-this-goes-into-the-release is absent.
👉 What to do: we're in the next Sourcegraph release code freeze period. If you are 100% sure your changes should get released or provide no risk to the release, add the label your PR with i-acknowledge-this-goes-into-the-release.

@github-actions

github-actions Bot commented Jan 13, 2023

Copy link
Copy Markdown
Contributor

❌ Problem: the label i-acknowledge-this-goes-into-the-release is absent.
👉 What to do: we're in the next Sourcegraph release code freeze period. If you are 100% sure your changes should get released or provide no risk to the release, add the label your PR with i-acknowledge-this-goes-into-the-release.

@github-actions

github-actions Bot commented Jan 13, 2023

Copy link
Copy Markdown
Contributor

❌ Problem: the label i-acknowledge-this-goes-into-the-release is absent.
👉 What to do: we're in the next Sourcegraph release code freeze period. If you are 100% sure your changes should get released or provide no risk to the release, add the label your PR with i-acknowledge-this-goes-into-the-release.

burmudar
burmudar commented Jan 13, 2023
View reviewed changes
Comment thread internal/database/schema.md Outdated
@github-actions

github-actions Bot commented Jan 13, 2023

Copy link
Copy Markdown
Contributor

❌ Problem: the label i-acknowledge-this-goes-into-the-release is absent.
👉 What to do: we're in the next Sourcegraph release code freeze period. If you are 100% sure your changes should get released or provide no risk to the release, add the label your PR with i-acknowledge-this-goes-into-the-release.

1 similar comment
@github-actions

github-actions Bot commented Jan 13, 2023

Copy link
Copy Markdown
Contributor

❌ Problem: the label i-acknowledge-this-goes-into-the-release is absent.
👉 What to do: we're in the next Sourcegraph release code freeze period. If you are 100% sure your changes should get released or provide no risk to the release, add the label your PR with i-acknowledge-this-goes-into-the-release.

mrnugget
mrnugget approved these changes Jan 16, 2023
View reviewed changes

@mrnugget mrnugget 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.

Good stuff, dude. Impressive, actually.

One thing I want though: tests that make sure we clean up the counters once the repos are recloned. See my comment on the test file.

Comment thread internal/database/repo_statistics_test.go Outdated
Comment thread migrations/frontend/1673351808_add_repo_corruption_stat/down.sql Outdated
Comment thread migrations/frontend/1673351808_add_repo_corruption_stat/up.sql Outdated
Comment thread migrations/frontend/1673351808_add_repo_corruption_stat/up.sql Outdated
Comment thread migrations/frontend/1673351808_add_repo_corruption_stat/up.sql Outdated
burmudar and others added 6 commits January 16, 2023 15:56
@burmudar
add corrupted stat to store and db
b7c76a8 
- update repo statistics tests for corrupted stat
@burmudar
make corrupted stat column NOT NULL
89175f6
@burmudar @sashaostrikov
set type of corrupted to bigint
8d01111 
- review comments
- set not null for corrupted in repo_statistics

Co-authored-by: Alex Ostrikov <alex.ostrikov@sourcegraph.com>
@burmudar
make corrupted column comment consistent
e0e302f
@burmudar @mrnugget
review feedback
e1b76c0 
- add testcase to simulate recloning
- fix comments of what changed in sql migration

Co-authored-by: Thorsten Ball <mrnugget@gmail.com>
@burmudar
run sg gen
07ca3d1
@burmudar burmudar force-pushed the wb/repo/corruption-filter-count branch from 894b36f to 07ca3d1 Compare January 16, 2023 13:57
@burmudar Graphite App

burmudar commented Jan 16, 2023 •
edited
Loading

Copy link
Copy Markdown
Contributor Author

⚠️ This pull request merged successfully, but the Graphite job failed before completion
Stack job ID: ANkhuAH5uGZvSNzCLWgo.
See details on graphite.dev

@burmudar burmudar merged commit 1ccd8e5 into main Jan 16, 2023
@burmudar burmudar deleted the wb/repo/corruption-filter-count branch January 16, 2023 14:36
@burmudar burmudar mentioned this pull request Feb 15, 2023
Closed
4 tasks
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Reviewers

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 i-acknowledge-this-goes-into-the-release

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@burmudar @sourcegraph-bot @mrnugget @sashaostrikov