tektoncd / results

Long term storage of execution results.
Apache License 2.0
78 stars 74 forks source link

WIP: address logs memory leak, but avoid canceled context on UpdateLog call #714

Closed gabemontero closed 9 months ago

gabemontero commented 9 months ago

Changes

multi-threaded alternative to https://github.com/tektoncd/results/pull/712

Submitter Checklist

These are the criteria that every PR should meet, please check them off as you review them:

Release Notes

tekton-robot commented 9 months ago

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: To complete the pull request process, please assign enarha after the PR has been reviewed. You can assign the PR to them by writing /assign @enarha in a comment when ready.

The full list of commands accepted by this bot can be found here.

Needs approval from an approver in each of these files: - **[OWNERS](https://github.com/tektoncd/results/blob/main/OWNERS)** Approvers can indicate their approval by writing `/approve` in a comment Approvers can cancel approval by writing `/approve cancel` in a comment
tekton-robot commented 9 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/watcher/reconciler/dynamic/dynamic.go 69.3% 63.4% -5.9
gabemontero commented 9 months ago

between our stress tests finding some issues with this form of the mem leak fix, and both our stress test and individual testing looking better with our preferred, reduce thread solution in #712 I'm closing this PR.