Closed gabemontero closed 3 months ago
The following is the coverage report on the affected files.
Say /test pull-tekton-results-go-coverage
to re-run this coverage report
File | Old Coverage | New Coverage | Delta |
---|---|---|---|
pkg/api/server/v1alpha2/logs.go | 63.4% | 63.2% | -0.2 |
pkg/watcher/reconciler/dynamic/dynamic.go | 55.3% | 53.0% | -2.3 |
The following is the coverage report on the affected files.
Say /test pull-tekton-results-go-coverage
to re-run this coverage report
File | Old Coverage | New Coverage | Delta |
---|---|---|---|
pkg/api/server/v1alpha2/logs.go | 63.4% | 63.2% | -0.2 |
pkg/watcher/reconciler/dynamic/dynamic.go | 55.3% | 54.5% | -0.8 |
[APPROVALNOTIFIER] This PR is APPROVED
This pull-request has been approved by: avinal
The full list of commands accepted by this bot can be found here.
The pull request process is described here
Changes
There was a great disparity between performance once we applied sufficient concurrency on production scale systems with log storage on the reconciler thread.
After a couple of weeks of analysis between myself and @pmacik, the primary scaling bottleneck was at the golang http level, specifically the http2 Body.Read(), at https://github.com/tektoncd/results/blob/ca1ca870efebf9e122684134a9d38862c0be2375/vendor/google.golang.org/grpc/internal/transport/handler_server.go#L420
For the time being then, that has forced us to move log storage back off of the log storage thread.
We will circle back to retry on recoverable errors once the Log status changes are in.
Details on the changes in this PR include:
/kind bug
@khrm @enarha PTAL
@sayan-biswas @avinal fyi / ptal if you all have cycles
Submitter Checklist
These are the criteria that every PR should meet, please check them off as you review them:
/kind <type>
. Valid types are bug, cleanup, design, documentation, feature, flake, misc, question, tepRelease Notes