refactor: Improvements to database querying, notify missing records#5527
Hidden character warning
refactor: Improvements to database querying, notify missing records#5527michaelkedar wants to merge 6 commits into
Conversation
| activeCount.Store(int64(len(subCtxs))) | ||
|
|
||
| for _, ctx := range subCtxs { | ||
| go func(c context.Context) { |
There was a problem hiding this comment.
Apparently there's this function called context.AfterFunc() which can be used here instead of us manually spawning a goroutine?
There was a problem hiding this comment.
This should result in a lot less goroutines because it'll all be managed by the parent context.
| BatchTimeout time.Duration | ||
| BatchMaxElements int |
There was a problem hiding this comment.
Do we want to make these configurable in main.go? (Probably as env vars?)
There was a problem hiding this comment.
added some env var parsing
| // We check if the goroutine for req1 finished quickly | ||
| c1Done := make(chan struct{}) |
There was a problem hiding this comment.
The comment say we check, but does this check? It only seems to be checking if c1Done is finished (both requests finished)
There was a problem hiding this comment.
test should be fixed now
| func (s *VulnerabilityStore) batchGetModified(ctx context.Context, ids []string) []batcher.Result[time.Time] { | ||
| results := make([]batcher.Result[time.Time], len(ids)) | ||
|
|
||
| keys := make([]*datastore.Key, len(ids)) |
There was a problem hiding this comment.
Is it worth deduping here?
There was a problem hiding this comment.
actually probably not worth the additional logic, though maybe add a comment to state that's something considered.
2 participants
gcs_missingmessage to the recoverer if a vuln matched in datastore, but no full records was found in GCS.GetModifiedqueries together across goroutines into a single datastoreGetMultirequest to save on network round trips.