Add concurrency control parameters #144
Conversation
ccampo133
force-pushed
the
add_concurrency_control_params
branch
2 times, most recently
from
deee4d2
to
7c1799f
Compare
ccampo133
force-pushed
the
add_concurrency_control_params
branch
from
7c1799f
to
ce8478f
Compare
sql/scanner.go
Outdated
| if sema != nil { | ||
| // Release the slot once the goroutine is done. | ||
| sema.Release(1) | ||
| } | ||
| wg.Done() |
There was a problem hiding this comment.
This can be done in a deferred call for more safety (from premature returns like the one you had earlier in case of ctx.Done()
There was a problem hiding this comment.
This function currently can't actually, return early, so the effect is the same, but I will add it to a defer just to to make it more resilient to possible future changes.
| // database handles (one per database). | ||
| var sema *semaphore.Weighted | ||
| if s.config.RepoConfig.MaxOpenConns > 0 { | ||
| sema = semaphore.NewWeighted(int64(s.config.RepoConfig.MaxOpenConns)) |
There was a problem hiding this comment.
What are we now doing with MaxOpenConns (is it needed any longer)?
There was a problem hiding this comment.
Yes, it is still needed. It eventually gets passed all the way down to the sql.DB instance - see: https://github.com/cyralinc/dmap/blob/main/sql/generic.go#L243
This ultimately means that the number of concurrent queries for a single database will be min(MaxOpenConns, MaxConcurrency), since all queries to a database use the same sql.DB instance.
Description of the change
This PR adds some concurrency control parameters which can be useful to tweaking performance/resource usage and dealing with long-running queries.
Type of change
Checklists
Development
Code review
Testing
Unit and manual testing.