gms,service: add a feature to protect the usage of allow_mutation_read_page_without_live_row #15890

denesb · 2023-10-31T06:55:21Z

allow_mutation_read_page_without_live_row is a new option in the partition_slice::option option set. In a mixed clusters, old nodes possibly don't know this new option, so its usage must be protected by a cluster feature. This patch does just that.

Fixes: #15795

scylladb-promoter · 2023-10-31T07:07:04Z

🔴 CI State: FAILURE

❌ - Build

Build Failure:

[2023-10-31T07:05:05.653Z] FAILED: build/debug/service/storage_proxy.o 
[2023-10-31T07:05:05.653Z] clang++ -MD -MT build/debug/service/storage_proxy.o -MF build/debug/service/storage_proxy.o.d -I/jenkins/workspace/scylla-master/scylla-ci/scylla/seastar/include -I/jenkins/workspace/scylla-master/scylla-ci/scylla/build/debug/seastar/gen/include -U_FORTIFY_SOURCE -Werror=unused-result -fstack-clash-protection -fsanitize=address -fsanitize=undefined -fno-sanitize=vptr -DSEASTAR_API_LEVEL=7 -DSEASTAR_BUILD_SHARED_LIBS -DSEASTAR_SSTRING -DSEASTAR_SCHEDULING_GROUPS_COUNT=16 -DSEASTAR_DEBUG -DSEASTAR_DEFAULT_ALLOCATOR -DSEASTAR_SHUFFLE_TASK_QUEUE -DSEASTAR_DEBUG_SHARED_PTR -DSEASTAR_LOGGER_TYPE_STDOUT -DSEASTAR_TYPE_ERASE_MORE -DBOOST_NO_CXX98_FUNCTION_BASE -DFMT_SHARED -I/usr/include/p11-kit-1   -ffile-prefix-map=/jenkins/workspace/scylla-master/scylla-ci/scylla=. -march=westmere -DDEBUG -DSANITIZE -DDEBUG_LSA_SANITIZER -DSCYLLA_ENABLE_ERROR_INJECTION -Og -DSCYLLA_BUILD_MODE=debug -g -gz -iquote. -iquote build/debug/gen --std=gnu++20  -ffile-prefix-map=/jenkins/workspace/scylla-master/scylla-ci/scylla=. -march=westmere -DBOOST_TEST_DYN_LINK   -DNOMINMAX -DNOMINMAX  -fvisibility=hidden -Wall -Werror -Wimplicit-fallthrough -Wno-mismatched-tags -Wno-c++11-narrowing -Wno-overloaded-virtual -Wno-unused-command-line-argument -Wno-unsupported-friend -Wno-implicit-int-float-conversion -Wno-psabi -Wno-narrowing -Wno-error=deprecated-declarations -DXXH_PRIVATE_API -DSEASTAR_TESTING_MAIN -DFMT_DEPRECATED_OSTREAM  -c -o build/debug/service/storage_proxy.o service/storage_proxy.cc
[2023-10-31T07:05:05.653Z] service/storage_proxy.cc:4984:13: error: use of undeclared identifier features; did you mean gms::feature?
[2023-10-31T07:05:05.654Z]         if (features().empty_replica_mutation_pages) {
[2023-10-31T07:05:05.654Z]             ^
[2023-10-31T07:05:05.654Z] ./db/system_keyspace.hh:58:11: note: gms::feature declared here
[2023-10-31T07:05:05.654Z]     class feature;
[2023-10-31T07:05:05.654Z]           ^
[2023-10-31T07:05:05.654Z] 1 error generated.

Build Details:

Build URL: https://jenkins.scylladb.com/job/scylla-master/job/scylla-ci/4452/
Duration: 11 min
Builder: spider5.cloudius-systems.com

scylladb-promoter · 2023-10-31T09:47:14Z

🔴 CI State: FAILURE

✅ - Build
❌ - Sanity Tests
❌ - Unit Tests

Failed Tests (27/21164):

Build Details:

Build URL: https://jenkins.scylladb.com/job/scylla-master/job/scylla-ci/4453/
Duration: 2 hr 20 min
Builder: spider5.cloudius-systems.com

avikivity · 2023-10-31T10:21:47Z

Aha, I remember that we sent the new bit without a feature thinking it will be ignored, but it's caught.

scylladb-promoter · 2023-10-31T13:41:50Z

🔴 CI State: FAILURE

✅ - Build
❌ - Sanity Tests
❌ - Unit Tests

Failed Tests (28/21164):

Build Details:

Build URL: https://jenkins.scylladb.com/job/scylla-master/job/scylla-ci/4466/
Duration: 2 hr 26 min
Builder: spider6.cloudius-systems.com

denesb · 2023-10-31T14:15:02Z

I don't understand all this failures on such a simple patch. Rebased.

avikivity · 2023-10-31T16:05:09Z

I don't understand all this failures on such a simple patch. Rebased.

Ran on m5ad.8xlarge, so should be okay disk-wise.

avikivity · 2023-10-31T16:11:23Z

Actually it ran on many instance types, but all have nvme as far as I can tell.

avikivity · 2023-10-31T16:12:00Z

@fruch is there any way to tell if the mount procedure worked correctly?

scylladb-promoter · 2023-10-31T16:46:29Z

🔴 CI State: FAILURE

✅ - Build
❌ - Sanity Tests
✅ - Unit Tests

Failed Tests (21/21164):

Build Details:

Build URL: https://jenkins.scylladb.com/job/scylla-master/job/scylla-ci/4470/
Duration: 2 hr 31 min
Builder: spider1.cloudius-systems.com

fruch · 2023-10-31T16:49:32Z

Now, we are not collecting logs from that stage.

Anyhow those failures are too reproducible, it doesn't fit the patterns we've seen when disk wasn't configured

avikivity · 2023-10-31T17:22:33Z

yes, looks like a genuine bug uncovered by CI (like it's supposed to)

denesb · 2023-11-01T07:50:24Z

Looks like a genuine bug related to view schemas. Nodes are crashing due to uninitialized schema: #15904

denesb · 2023-11-01T07:54:48Z

All the failing tests I checked so far are related to schema problems.

denesb · 2023-11-03T06:56:21Z

DO NOT MERGE!!!!

I pushed a version which has the if condition commented, to see if that is what triggers all these failures. The patch now effectively just adds a feature flag.

scylladb-promoter · 2023-11-03T09:27:17Z

🔴 CI State: FAILURE

✅ - Build
❌ - Sanity Tests
❌ - Unit Tests

Failed Tests (27/21268):

Build Details:

Build URL: https://jenkins.scylladb.com/job/scylla-master/job/scylla-ci/4503/
Duration: 2 hr 31 min
Builder: spider7.cloudius-systems.com

kbr-scylla · 2023-11-03T10:35:46Z

I looked at the test_topology_ops failure and created #15935.

There are a couple of bugs that have already been detected by the test. For now I'll disable the part of the test which is detecting these issues (the one which is performing CQL writes) until we fix all known bugs.

denesb · 2023-11-03T10:38:18Z

I looked at the test_topology_ops failure and created #15935.

There are a couple of bugs that have already been detected by the test. For now I'll disable the part of the test which is detecting these issues (the one which is performing CQL writes) until we fix all known bugs.

Thanks. Why is this particular PR triggering this so reliably? I also saw this test fail at other occasions, but here it is very consistent.

denesb · 2023-11-03T10:39:12Z

Thanks. Why is this particular PR triggering this so reliably? I also saw this test fail at other occasions, but here it is very consistent.

Actually, it is not that consistent.

scylladb-promoter · 2023-11-03T13:00:43Z

🔴 CI State: FAILURE

✅ - Build
❌ - Sanity Tests
❌ - Unit Tests

Failed Tests (28/21268):

Build Details:

Build URL: https://jenkins.scylladb.com/job/scylla-master/job/scylla-ci/4504/
Duration: 2 hr 23 min
Builder: spider4.cloudius-systems.com

scylladb-promoter · 2023-11-07T13:55:39Z

🔴 CI State: FAILURE

✅ - Build
❌ - Sanity Tests
❌ - Unit Tests

Failed Tests (24/21278):

Build Details:

Build URL: https://jenkins.scylladb.com/job/scylla-master/job/scylla-ci/4568/
Duration: 2 hr 23 min
Builder: spider7.cloudius-systems.com

denesb · 2023-11-07T14:53:02Z

I have no other choice but to dig into these failures. Seems like there is a real problem behind all this.

denesb · 2023-11-08T13:50:39Z

test_simple_kill_node_while_decommissioning 🔍

#15146

denesb · 2023-11-08T13:57:14Z

topology_experimental_raft.test_topology_ops.dev.2 🔍
topology_experimental_raft.test_topology_ops.dev.3 🔍

#15924

kbr-scylla · 2023-11-08T14:15:05Z

#15924

Rebase -- these background writes are disabled on master (6cc5bca)

denesb · 2023-11-08T14:34:00Z

test_off_strategy_during_bootstrap 🔍

#16004

denesb · 2023-11-08T14:37:36Z

test_cluster_cleanup 🔍

#16004

denesb · 2023-11-08T14:42:12Z

test_tracing_shutdown 🔍

#16004 with a twist. Writes to system_traces fail to lookup the table by version (due to different version), so they call back to the originating node, which is shutting down and has already unregistered its RPC verbs. Not sure if this deserves a separate bug or not.

denesb · 2023-11-08T14:47:52Z

Looks like almost all failed tests are related to #16004.

denesb · 2023-11-08T14:51:50Z

#15924

Rebase -- these background writes are disabled on master (6cc5bca)

Rebased.

scylladb-promoter · 2023-11-08T17:13:32Z

🔴 CI State: FAILURE

✅ - Build
❌ - Sanity Tests
❌ - Unit Tests

Failed Tests (23/21824):

Build Details:

Build URL: https://jenkins.scylladb.com/job/scylla-master/job/scylla-ci/4610/
Duration: 2 hr 25 min
Builder: spider4.cloudius-systems.com

…d_page_without_live_row allow_mutation_read_page_without_live_row is a new option in the partition_slice::option option set. In a mixed clusters, old nodes possibly don't know this new option, so its usage must be protected by a cluster feature. This patch does just that. Fixes: scylladb#15795

denesb · 2023-11-16T15:52:21Z

Rebased on top of 2238144 (the fix for #16004).

bhalevy · 2023-11-16T16:23:56Z

Rebased on top of 2238144 (the fix for #16004).

Cc @mykaul

scylladb-promoter · 2023-11-16T19:07:56Z

🟢 CI State: SUCCESS

✅ - Build
✅ - Unit Tests
✅ - Sanity Tests

Build Details:

Build URL: https://jenkins.scylladb.com/job/scylla-master/job/scylla-ci/4741/
Duration: 2 hr 49 min
Builder: spider2.cloudius-systems.com

bhalevy · 2023-11-16T19:10:56Z

@kbr-scylla / @avikivity do you need to re-review or can this PR be merged as is?

kbr-scylla

I'd like at least one other person who knows this code, preferably someone who reviewed the previous PR (which extended the enum_set), to review this one before I merge.

denesb · 2023-11-17T10:48:01Z

I'd like at least one other person who knows this code, preferably someone who reviewed the previous PR (which extended the enum_set), to review this one before I merge.

That person is @tgrabiec.

…d_page_without_live_row allow_mutation_read_page_without_live_row is a new option in the partition_slice::option option set. In a mixed clusters, old nodes possibly don't know this new option, so its usage must be protected by a cluster feature. This patch does just that. Fixes: #15795 Closes #15890 (cherry picked from commit f539612)

denesb requested a review from tgrabiec as a code owner October 31, 2023 06:55

denesb mentioned this pull request Oct 31, 2023

read load failing after one node upgrade [bad_enum_set_mask (Bit mask contains invalid enumeration indices.)] #15795

Closed

2 tasks

denesb requested a review from kbr-scylla October 31, 2023 07:01

denesb force-pushed the allow-empty-pages-cluster-feature branch from 77137be to 8e15394 Compare October 31, 2023 07:25

denesb requested a review from avikivity October 31, 2023 09:07

denesb force-pushed the allow-empty-pages-cluster-feature branch from 8e15394 to 317fce1 Compare October 31, 2023 14:14

denesb mentioned this pull request Nov 1, 2023

[dtest] secondary_indexes_test.py::TestSecondaryIndexes: abort due to uninitialized base schema #15904

Closed

denesb force-pushed the allow-empty-pages-cluster-feature branch from 317fce1 to 7af279a Compare November 3, 2023 06:55

denesb force-pushed the allow-empty-pages-cluster-feature branch 2 times, most recently from e09527e to ea9c8c7 Compare November 3, 2023 10:37

denesb force-pushed the allow-empty-pages-cluster-feature branch from ea9c8c7 to b5a4ad8 Compare November 7, 2023 11:08

denesb force-pushed the allow-empty-pages-cluster-feature branch from b5a4ad8 to 1b87d9c Compare November 8, 2023 14:47

denesb mentioned this pull request Nov 8, 2023

migration_manager: schema version correctness depends on order of feature enabling #16004

Closed

denesb mentioned this pull request Nov 10, 2023

migration_manager: also reload schema on enabling digest_insensitive_to_expiry #16013

Closed

denesb force-pushed the allow-empty-pages-cluster-feature branch from 1b87d9c to 903c2a1 Compare November 16, 2023 15:51

kbr-scylla approved these changes Nov 17, 2023

View reviewed changes

tgrabiec approved these changes Nov 20, 2023

View reviewed changes

scylladb-promoter closed this in f539612 Nov 20, 2023

piodul mentioned this pull request Nov 22, 2023

raft topology: assigning tokens races with setup_group0, causing data path errors #15935

Closed

gms,service: add a feature to protect the usage of allow_mutation_read_page_without_live_row #15890

gms,service: add a feature to protect the usage of allow_mutation_read_page_without_live_row #15890

Conversation

denesb commented Oct 31, 2023

scylladb-promoter commented Oct 31, 2023

🔴 CI State: FAILURE

Build Failure:

Build Details:

scylladb-promoter commented Oct 31, 2023

🔴 CI State: FAILURE

Failed Tests (27/21164):

Build Details:

avikivity commented Oct 31, 2023

scylladb-promoter commented Oct 31, 2023

🔴 CI State: FAILURE

Failed Tests (28/21164):

Build Details:

denesb commented Oct 31, 2023

avikivity commented Oct 31, 2023

avikivity commented Oct 31, 2023

avikivity commented Oct 31, 2023

scylladb-promoter commented Oct 31, 2023

🔴 CI State: FAILURE

Failed Tests (21/21164):

Build Details:

fruch commented Oct 31, 2023

avikivity commented Oct 31, 2023

denesb commented Nov 1, 2023

denesb commented Nov 1, 2023

denesb commented Nov 3, 2023

scylladb-promoter commented Nov 3, 2023

🔴 CI State: FAILURE

Failed Tests (27/21268):

Build Details:

kbr-scylla commented Nov 3, 2023

denesb commented Nov 3, 2023

denesb commented Nov 3, 2023

scylladb-promoter commented Nov 3, 2023

🔴 CI State: FAILURE

Failed Tests (28/21268):

Build Details:

scylladb-promoter commented Nov 7, 2023

🔴 CI State: FAILURE

Failed Tests (24/21278):

Build Details:

denesb commented Nov 7, 2023

denesb commented Nov 8, 2023

denesb commented Nov 8, 2023

kbr-scylla commented Nov 8, 2023

denesb commented Nov 8, 2023

denesb commented Nov 8, 2023 • edited

denesb commented Nov 8, 2023

denesb commented Nov 8, 2023

denesb commented Nov 8, 2023

scylladb-promoter commented Nov 8, 2023

🔴 CI State: FAILURE

Failed Tests (23/21824):

Build Details:

denesb commented Nov 16, 2023

bhalevy commented Nov 16, 2023

scylladb-promoter commented Nov 16, 2023

🟢 CI State: SUCCESS

Build Details:

bhalevy commented Nov 16, 2023

kbr-scylla left a comment

Choose a reason for hiding this comment

denesb commented Nov 17, 2023

denesb commented Nov 8, 2023 •

edited