Context fixes #2930
Context fixes #2930
Conversation
1. There is a number of related contextcheck warnings: pkg/services/control/server/gc.go:50:29 contextcheck Function `Delete->Delete$1->delete->inhumeAddr->Inhume` should pass the context parameter pkg/local_object_storage/engine/inhume.go:329:20 contextcheck Function `Inhume->Inhume$1->inhume->inhumeAddr->Inhume` should pass the context parameter pkg/services/policer/check.go:95:44 contextcheck Function `Inhume->Inhume$1->inhume->inhumeAddr->Inhume` should pass the context parameter which tell us that the context chain is not preserved correctly, we're resorting to some background context in all cases. 2. context.Background() means that this context is totally useless and the relevant context-related logic is never triggered. 3. Consider we've passed a proper context here and iteration can really be stopped. This would mean we can end up in an inconsistent state since lock object will be gone, but some shards may still have not performed this cleanup. It's safer to just drop this context, this function must run to completion. It's local, no networking involved. Signed-off-by: Roman Khimov <[email protected]>
pkg/services/tree/replicator.go:97:3 contextcheck Function `replicationWorker->replicationWorker$1` should pass the context parameter Signed-off-by: Roman Khimov <[email protected]>
Fixes contextcheck: pkg/services/policer/check.go:132:17 contextcheck Function `processNodes` should pass the context parameter Signed-off-by: Roman Khimov <[email protected]>
Linter: pkg/services/object/search/search.go:36:14 contextcheck Function `execute->analyzeStatus->executeOnContainer` should pass the context parameter This also fixes a bug in executeOnContainer() since internal calls to c.searchObjects should use the inner context, not outer one. Signed-off-by: Roman Khimov <[email protected]>
pkg/services/object/get/get.go:91:14 contextcheck Function `execute->analyzeStatus->assemble->processV2Split->processV2Link->getChild` should pass the context parameter Can be untangled, but we a lot of calls here joined by the same context, so I'd keep it as is for now. Signed-off-by: Roman Khimov <[email protected]>
roman-khimov
requested review from
carpawell and
cthulhu-rider
as code owners
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #2930 +/- ##
==========================================
- Coverage 23.88% 23.87% -0.01%
==========================================
Files 775 775
Lines 45610 45604 -6
==========================================
- Hits 10892 10888 -4
+ Misses 33861 33859 -2
Partials 857 857 ☔ View full report in Codecov by Sentry. |
There was a problem hiding this comment.
OK to me overall, but what this PR is about? contextcheck appears out of nowhere (according to commits).
Consider we've passed a proper context here and iteration can really be
stopped. This would mean we can end up in an inconsistent state since lock
object will be gone, but some shards may still have not performed this
cleanup.
Consider you are trying to stop a node and it is freezed for minutes. Will you try to kill it? It is not about this case, I worry in general. Are you sure you made it better if it tries to finish does not matter how long it takes?
|
How would you stop it there? It can take any amount of time per shard anyway. The PR itself is related to nspcc-dev/.github#34 which brings the question of #1911 which also brought me to #2929. |
Yes, sure, I do understand it but only now and only when I saw recently merged PRs in another repos. In history it will fix the linter not even presented in our code. |
No description provided.