srds: remove scope from scope_name_by_hash_ in case the scope key changes #36702
Conversation
…nges Signed-off-by: Eugene Chan <[email protected]>
|
Thanks please add a release note for this fix. /wait |
Signed-off-by: Eugene Chan <[email protected]>
|
/retest |
|
/retest |
1 similar comment
|
/retest |
|
Can you try merging main? CI has been undergoing a lot of changes lately. |
Signed-off-by: pianiststickman <[email protected]>
Head branch was pushed to by a user without write access
|
@phlax we can't seem to get this to pass CI? What is the recommended action? |
|
looking ... |
|
main merge should fix the checks/tsan issue the other needs to switch |
|
ill raise a PR now for the kv store test ... |
|
landed - |
|
coverage fail is real, but not 100% if your PR is cause /coverage |
|
Coverage for this Pull Request will be rendered here: https://storage.googleapis.com/envoy-pr/36702/coverage/index.html The coverage results are (re-)rendered each time the CI |
|
/retest cache fail |
|
I think (hope?) it's unrelated, CI all passed when I opened this PR :) /retest |
|
apologies - we migrated to a new RBE backend and its been a case of juggling the worker allocations - it should be getting more stable now |
|
No worries, thanks for all your help on this! |
|
@mattklein123 could you re-approve and merge please? Thank you! |
Merge artifact - we're checking this condition above, so this block should never be executed. However, the logic was just tightened in PR envoyproxy#36702 and isn't reflected in this block, so removing it. Signed-off-by: Eugene Chan <[email protected]>
An extraneous if block was introduced merging #36702 into #36703. We're checking this condition above in L282, so this block should never be executed. Commit Message: remove a redundant if block Additional Description: n/a Risk Level: low Testing: existing CI Docs Changes: n/a Release Notes: n/a Platform Specific Features: n/a Signed-off-by: Eugene Chan <[email protected]>
Commit Message: srds: remove scope from scope_name_by_hash_ in case the scope key changes
Additional Description:
When a SRDS resource is updated such that its scope key changes, the scope_name_by_hash_ map used to detect scope key conflicts keeps the resource around under the old hash. This may cause spurious conflicts if the resource is removed and/or a resource is added with the old key, as demonstrated in the included tests.
Risk Level: low
Testing: CI, also added a DeltaXDS version of the test
Docs Changes: n/a
Release Notes: n/a
Platform Specific Features: n/a