search

crate / crate-qa

CrateDB Quality Assurance
4 stars 1 forks source link

Add sys.nodes query to rolling upgrade test #319

Closed BaurzhanSakhariev closed 1 month ago

BaurzhanSakhariev commented 2 months ago

Catches regression caused by https://github.com/crate/crate/commit/0653a1f99fbb14574d607e90f4a5523726ff7479

BaurzhanSakhariev commented 2 months ago

While I'm working on a fix (and won't merge this PR until I merge fix on master), how do we handle possible 5.7.x -> 5.8.0?

AFAIK, UpgradePath('5.7.x', '5.8.x') doesn't exclude 5.8.0, it gets random version from 5.8.X series? Even if we have working 5.8.1, we can end up with a flaky test?

matriv commented 2 months ago

cr8 which is used underneath, uses the latest hotfix version from the series, when you use .x:

cr8 run-crate 5.6.x 
Downloading https://cdn.crate.io/downloads/releases/cratedb/x64_linux/crate-5.6.5.tar.gz and extracting to /home/matriv/.cache/cr8/crates

(Always downloads 5.6.5)

BaurzhanSakhariev commented 2 months ago

uses the latest hotfix version from the series

Thanks for the clarification, so no flaky tests then. I will merge this after releasing 5.8.1 in order to avoid nighthly job failures

matriv commented 2 months ago

We could also try a run using branch:5.8 instead of 5.8.x once the fix is merged.

matriv commented 2 months ago

retest this please

BaurzhanSakhariev commented 1 month ago

That's not really valid (ie such code doesn't really ensure that handler node is on version before upgrade).

Request can sometimes hit replica and I don't want to set replicas to 0 as it might be important for other tests. Given that we already reproduced issue with this change and also verified that it works with it, I will close it.

matriv commented 1 month ago

sys.nodes don't have shards so I think it's deterministic it will hit an old node which will ask to get the info from an upgraded node. So I think it's valuable to add and catch such regressions early in the future.

BaurzhanSakhariev commented 1 month ago

sys.nodes don't have shards

right, reopened. thx for catching!