Conversation
enterprise/cmd/frontend/internal/licensing/enforcement/users.go
Outdated
Show resolved
Hide resolved
20c3554 to
545b3bf
Compare
d30ca74 to
2c821ad
Compare
|
❌ Problem: the label |
|
Codenotify: Notifying subscribers in CODENOTIFY files for diff 50c9fb6...69c5421.
|
| return nil | ||
| } | ||
|
|
||
| numPrivateRepos, err := s.RepoStore().Count(ctx, ossDB.ReposListOptions{OnlyPrivate: true}) |
There was a problem hiding this comment.
Don't know if this is true:
What about we do the SQL query like:
SELECT COUNT(*) >= $1 FROM repo WHERE privateIn the hope that the PostgreSQL is smart enough to stop counting once it for sure knows the return value will be TRUE?
The benefit of doing it this way:
- We would not be so concerned about a large number of public repos in the table slowing down the query too much.
- Kinda doing better at avoiding race conditions like creating tons of repos in a short amount of time.
Maybe this is not worth it at all, given the assumption that repo-number-restricted instance would only have a small number of repos after all. 😅
There was a problem hiding this comment.
Hmmm might be worth it 🤔 Also not sure if Postgres would optimise this in some way. Let me try and find out
There was a problem hiding this comment.
I can't find evidence that this would be made any faster. But yes, there should not be a large amount of private repositories, and the only time this could be slow if someone is changing from a license where private repos is allowed, and they have a large amount of private repositories, to a license where it is restricted. And then only in the transitioning period could it maybe be slow
There was a problem hiding this comment.
Agreed, I don't think it is worth optimizing the query too much, since it's OK to penalize people with lots of private repos who exceed the license.
| return nil | ||
| } | ||
|
|
||
| numPrivateRepos, err := s.RepoStore().Count(ctx, ossDB.ReposListOptions{OnlyPrivate: true}) |
There was a problem hiding this comment.
Agreed, I don't think it is worth optimizing the query too much, since it's OK to penalize people with lots of private repos who exceed the license.
Closes #45640
Only intended to be merged once we want the new free license to go live.
Test plan
Added unit tests where appropriate. I did manual tests as well.