sciond: Change refresh mechanism to be safer

[lukedirtwalker]

Instead of deleting next query entries, just always request at the PS.
This can prevent a weird interaction between concurrent requests, which would lead to 0 cached paths.

Marked as breaking because the behavior changes, i.e., sciond no longer deletes anything in the DB on the refresh flag.

Fixes #2871
Fixes #3416

---

This change is 

[scrye]

Reviewed 8 of 8 files at r1.
Reviewable status: all files reviewed, 2 unresolved discussions (waiting on @lukedirtwalker)

a discussion (no related file):
Should we mark this as a Breaking Change? While the behavior is similar to what applications using the Refresh flag expect, it's still a behavior change.

---

---

go/lib/infra/modules/segfetcher/resolver.go, line 333 at r1 (raw file):

		return false, err
	}
	return len(segs) == 0 && filtered > 0, nil

I'm missing something here, and it's probably due to my lack of familiarity with the code.

Reading this function, if len(segs) == 0, it means the slice is empty. Therefore, counting what is getting filtered out of should always return 0? In what scenario is this expected to evaluate to true?

[lukedirtwalker]

Reviewable status: all files reviewed, 2 unresolved discussions (waiting on @scrye)

a discussion (no related file):

Previously, scrye (Sergiu Costea) wrote…

Should we mark this as a Breaking Change? While the behavior is similar to what applications using the Refresh flag expect, it's still a behavior change.

We can yes. Also updated the comment.

---

---

go/lib/infra/modules/segfetcher/resolver.go, line 333 at r1 (raw file):

segs.FilterSegsErr

modifies the slice inplace

[scrye]

Reviewable status: complete! all files reviewed, all discussions resolved

---

go/lib/infra/modules/segfetcher/resolver.go, line 333 at r1 (raw file):

Previously, lukedirtwalker (Lukas Vogel) wrote…

segs.FilterSegsErr

modifies the slice inplace

Ah, gotcha.