-
Notifications
You must be signed in to change notification settings - Fork 3.8k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
roachtest: acceptance/cli/node-status failed #51497
Comments
(roachtest).acceptance/cli/node-status failed on master@af0031e3004327b8d09e23e99eb9659abf7d82de:
More
Artifacts: /acceptance/cli/node-status
See this test on roachdash |
(roachtest).acceptance/cli/node-status failed on master@d79679f3e4e86d3f85c60f5431b8d5874a95735e:
More
Artifacts: /acceptance/cli/node-status
See this test on roachdash |
(roachtest).acceptance/cli/node-status failed on master@b901074cd8c2b115affb2b8f5dd89d84e5cf6e32:
More
Artifacts: /acceptance/cli/node-status
See this test on roachdash |
(roachtest).acceptance/cli/node-status failed on master@a0123f1bc050f67b942ff1e36181847f0edb3e10:
More
Artifacts: /acceptance/cli/node-status
See this test on roachdash |
(roachtest).acceptance/cli/node-status failed on master@8354896f7aa141132765c366ae7ddce9e9e7f361:
More
Artifacts: /acceptance/cli/node-status
See this test on roachdash |
Found the cause of this: the Due to recent changes in SQL executions, the rows are not ordered any more. The command o therwise appears to report the values expected by the test. The CLI code must be changed to sort the rows, or the test must be changed to expect the rows out of order. cc @tbg for triage and prioritization. I might suggest this as a task for @irfansharif if irfan is interested? |
Hm, strange, I see that the cli code already sorts by node id. Lines 227 to 229 in f10ba17
|
Huh, this is a bit bizzare.
It is sorting by node ID, but for n3 it thinks its address is 10.128.15.219. But the node stopped in this test is n2, which looking at the logs, is in fact stopped. But the logs are further confusing still. Looking at 1.logs/n1
Looking at 3.logs/n3 (??)
We've crossed wires at some point, and are confusing n2 and n3 for each other (probably in the test code). |
(This repros pretty readily.) |
I think I've broken |
(roachtest).acceptance/cli/node-status failed on master@e9a4f83e3eee59510f97db2c6e0df9b57cf6b944:
More
Artifacts: /acceptance/cli/node-status See this test on roachdash |
Brief update here: just rewrote |
(Also it was because I'd broken |
This is quite the workhorse, and does a lot and has to be compatible with a lot of existing CRDB versions. It's grown organically as a result and I'm finding it a bit difficult to maintain, breaking it down a bit makes it clearer what the structure of it all is and would've perhaps prevented me introducing bugs like cockroachdb#51497. Do scrutinize the PR closely, we use `roachprod start` everywhere. It's mostly mindless code movement but I did sneak in the fix for cockroachdb#51497 where I'd broken node ID assignments for when `roachprod start` is called with the `--sequential` flag (true by default). I did so by explicitly initializing the first node, and then having the remaining nodes join on to it. Release note: None
(roachtest).acceptance/cli/node-status failed on master@b8a50cc4d062293915969cdc83e3ec4d057cede5:
More
Artifacts: /acceptance/cli/node-status See this test on roachdash |
51243: kv/kvserver: clarify declared keys for RequestLease request r=nvanbenschoten a=nvanbenschoten The request type does not acquire latches but does still declare keys, which was confusing without an accompanying comment. 51525: sql: handle UPSERTs for partial indexes r=mgartner a=mgartner This commit allows partial indexes to maintain a consistent state when `UPSERT` statements are issued on the table. There were some structural changes made in order to better facilitate this functionality. First, `optbuilder` now keeps track of the column IDs of synthesized partial index predicate columns rather than ordinals. This simplifies the complex scoping logic needed for `UPSERT`s. Second, this commit introduces the PartialIndexUpdateManager which helps track which partial indexes need to be updated for a given row. Instead of passing around two `util.FastIntSet`s, this single struct is now used. It also de-duplicates code for interpretting the synthesized partial index predicate columns. Fixes #50222 Release note: None 51790: roachprod: rewrite `roachprod start` r=irfansharif a=irfansharif This is quite the workhorse, and does a lot and has to be compatible with a lot of existing CRDB versions. It's grown organically as a result and I'm finding it a bit difficult to maintain, breaking it down a bit makes it clearer what the structure of it all is and would've perhaps prevented me introducing bugs like #51497. Do scrutinize the PR closely, we use `roachprod start` everywhere. It's mostly mindless code movement but it's pretty fragile code. Release note: None 51858: builtins: add ST_RelatePattern r=rytaft a=otan Release note (sql change): Add the ST_RelatePattern builtin, which returns whether a given DE-9IM intersection matrix matches a given pattern. Co-authored-by: Nathan VanBenschoten <[email protected]> Co-authored-by: Marcus Gartner <[email protected]> Co-authored-by: irfan sharif <[email protected]> Co-authored-by: Oliver Tan <[email protected]>
(roachtest).acceptance/cli/node-status failed on master@bfa6307c292ef4dfed4a53cb99f506e6dab26533:
More
Artifacts: /acceptance/cli/node-status See this test on roachdash |
I believe this has failed after the roachprod update. Irfan mind having a look? |
The roachprod update didn't actually land the fix, it just landed the rewrite of the |
..and the setting of cluster settings for single node clusters. `roachprod start --sequential` was broken in cockroachdb#51329, and the broken-ness outlined in TODOs in cockroachdb#51790. This PR just addresses those TODOs. Fixes cockroachdb#51497 Fixes cockroachdb#51721 Fixes cockroachdb#51738 Fixes cockroachdb#51768 Fixes cockroachdb#51769 Fixes cockroachdb#51776 Release note: None
51893: roachprod: fixup `roachprod --sequential` r=irfansharif a=irfansharif ..and the setting of cluster settings for single node clusters. `roachprod start --sequential` was broken in #51329, and the broken-ness outlined in TODOs in #51790. This PR just addresses those TODOs. Fixes #51497 Fixes #51721 Fixes #51738 Fixes #51768 Fixes #51769 Fixes #51776 Release note: None Co-authored-by: irfan sharif <[email protected]>
(roachtest).acceptance/cli/node-status failed on master@3fba3b99d7164f3b8efb9fd8432b7749120d09c5:
More
Artifacts: /acceptance/cli/node-status
See this test on roachdash
powered by pkg/cmd/internal/issues
The text was updated successfully, but these errors were encountered: