Skip to content

Conversation

@julio-lopez
Copy link
Collaborator

@julio-lopez julio-lopez commented Jan 22, 2025

The kopia server was not uploading any logs to the repository, because "repodiag" blob uploads would always fail.

The cause was the following: When the (log) repodiag blob PUT operation was initiated, the Context used for this operation was already canceled.

The context used for blob uploads is passed to repodiag.NewLogManager when opening the repository. In the case of the kopia server, the repository is asynchronously opened in server.Server.InitReposotoryAsync. The context passed to repo.Open is canceled after the "open repository" server task completes. This issue was introduced in #1691

Change:
Use context.WithoutCancel() instead of the context passed when the repo "diagnoser" is created.

New tests are included to reproduce this failure and verify the fix.

Ref:

Thanks to:

  • 🎖️ @redgoat650 for reporting this issue;
  • 🎖️ @aaron-kasten for the triaging, testing and reproduction of the issue.

The kopia server was not uploading any logs to the repository,
because _"repodiag"_ blob uploads would always fail.

The cause was the following:
When the (log) repodiag blob PUT operation was initiated,
the `Context` used for this operation was already canceled.

The context used for blob uploads is passed to
`repodiag.NewLogManager` when opening the repository.
In the case of the kopia server, the repository is asynchronously
opened in `server.Server.InitReposotoryAsync`. The context
passed to `repo.Open` is canceled after the "open repository"
server task completes. This issue was introduced in kopia#1691

Change:
Use `context.WithoutCancel()` instead of the context passed
when the repo diagnoser is created.

New tests are included to reproduce this failure and verify
the fix.
@julio-lopez julio-lopez force-pushed the fix/repodiag-cancelled-context branch from adcbfde to 954dee4 Compare January 22, 2025 23:59
@codecov
Copy link

codecov bot commented Jan 23, 2025

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 76.30%. Comparing base (cb455c6) to head (954dee4).
Report is 401 commits behind head on master.

Additional details and impacted files
@@            Coverage Diff             @@
##           master    #4358      +/-   ##
==========================================
+ Coverage   75.86%   76.30%   +0.43%     
==========================================
  Files         470      527      +57     
  Lines       37301    39920    +2619     
==========================================
+ Hits        28299    30460    +2161     
- Misses       7071     7451     +380     
- Partials     1931     2009      +78     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@julio-lopez julio-lopez marked this pull request as ready for review January 23, 2025 00:28
@julio-lopez julio-lopez enabled auto-merge (squash) January 23, 2025 00:29
Copy link
Contributor

@redgoat650 redgoat650 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good; thanks 🚀

@julio-lopez
Copy link
Collaborator Author

FYI @jkowalski

@julio-lopez julio-lopez disabled auto-merge January 23, 2025 04:11
@julio-lopez julio-lopez merged commit 2a22281 into kopia:master Jan 23, 2025
28 checks passed
@julio-lopez julio-lopez deleted the fix/repodiag-cancelled-context branch January 23, 2025 04:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants