Fix maintenance node processing in policer #1604

fyrchik · 2025-01-16T13:37:05Z

fyrchik commented

2025-01-16 13:37:05 +00:00

There is a minor refactoring (aka simplification) in progress, but I may do this in another PR.

Consider REP 1 REP 1 placement (selects/filters are omitted).
The placement is [1, 2], [1, 0]. We are the 0-th node.
Node 1 is under maintenance, so we do not replicate object
on the node 2. In the second replication group node 1 is under maintenance,
but current caching logic considers it as "replica holder" and removes
local copy. Voilà, we have DL if the object is missing from the node 1.

TBD: write testing scenario for QA

There is a minor refactoring (aka simplification) in progress, but I may do this in another PR. Consider `REP 1 REP 1` placement (selects/filters are omitted). The placement is `[1, 2], [1, 0]`. We are the 0-th node. Node 1 is under maintenance, so we do not replicate object on the node 2. In the second replication group node 1 is under maintenance, but current caching logic considers it as "replica holder" and removes local copy. Voilà, we have DL if the object is missing from the node 1. TBD: write testing scenario for QA

fyrchik added 5 commits 2025-01-16 13:37:05 +00:00

policer: Use status instead of bool value in node cache b8113bcb36

Signed-off-by: Evgenii Stratonikov <e.stratonikov@yadro.com>

policer: Remove one-line helpers 8c3cc8002d

Signed-off-by: Evgenii Stratonikov <e.stratonikov@yadro.com>

policer: Do not process the same node twice 96a30c0353

Signed-off-by: Evgenii Stratonikov <e.stratonikov@yadro.com>

policer/test: Add test for MAINTENANCE runtime status 25ae681fd3

The node can have MAINTENANCE status in the network map, but can also be
ONLINE while responding with MAINTENANCE. These are 2 different code
paths, let's test them separately.

Signed-off-by: Evgenii Stratonikov <e.stratonikov@yadro.com>

policer: Properly handle maintenance nodes

DCO action / DCO (pull_request) Failing after 31s

Details

Tests and linters / Run gofumpt (pull_request) Successful in 28s

Details

Vulncheck / Vulncheck (pull_request) Successful in 1m17s

Details

Pre-commit hooks / Pre-commit (pull_request) Successful in 1m50s

Details

Build / Build Components (pull_request) Successful in 2m11s

Details

Tests and linters / Staticcheck (pull_request) Successful in 2m8s

Details

Tests and linters / Lint (pull_request) Successful in 3m5s

Details

Tests and linters / gopls check (pull_request) Successful in 5m3s

Details

Tests and linters / Tests (pull_request) Successful in 5m46s

Details

Tests and linters / Tests with -race (pull_request) Successful in 5m44s

Details

c47b639fe3

Signed-off-by: Evgenii Stratonikov <e.stratonikov@yadro.com>

fyrchik requested review from storage-core-committers 2025-01-16 13:37:06 +00:00

fyrchik requested review from storage-core-developers 2025-01-16 13:37:06 +00:00

fyrchik force-pushed fix-policer from c47b639fe3 to 0ab7818c81

2025-01-16 13:37:24 +00:00

Compare

fyrchik force-pushed fix-policer from 0ab7818c81 to 57efa0bc8e

2025-01-16 13:40:07 +00:00

Compare

fyrchik added the

bug

label 2025-01-16 13:40:25 +00:00

fyrchik reviewed 2025-01-16 13:42:59 +00:00

pkg/services/policer/policer_test.go

					
				@ -127,7 +128,7 @@ func TestProcessObject(t *testing.T) {

							nodeCount:       2,

							policy:          `REP 2 REP 2`,

							placement:       [][]int{{0, 1}, {0, 1}},

							wantReplicateTo: []int{1, 1}, // is this actually good?