From 63fcd5d312dee661c5605be5331fdf4c3c92c3ec Mon Sep 17 00:00:00 2001
From: Vartan Babayan <v.babayan@picodata.io>
Date: Mon, 2 Dec 2024 23:25:31 +0300
Subject: [PATCH] test: add proper test for pico.abort_ddl
---
src/governor/mod.rs | 2 ++
src/traft/node.rs | 1 +
test/int/test_ddl.py | 83 +++++++++++++++++++++++++++++++++++++++++---
3 files changed, 81 insertions(+), 5 deletions(-)
diff --git a/src/governor/mod.rs b/src/governor/mod.rs
index 53ba9aab12..3fa194e8a7 100644
--- a/src/governor/mod.rs
+++ b/src/governor/mod.rs
@@ -607,9 +607,11 @@ impl Loop {
}
});
}
+
let res = try_join_all(fs).await;
if let Err(OnError::Abort(cause)) = res {
next_op = Op::DdlAbort { cause };
+ crate::error_injection!(block "BLOCK_GOVERNOR_BEFORE_DDL_ABORT");
return Ok(());
}
diff --git a/src/traft/node.rs b/src/traft/node.rs
index 649d8942c0..385ec758a2 100644
--- a/src/traft/node.rs
+++ b/src/traft/node.rs
@@ -1140,6 +1140,7 @@ impl NodeImpl {
.expect("storage should not fail");
}
Op::DdlAbort { .. } => {
+ crate::error_injection!(block "BLOCK_GOVERNOR_BEFORE_DDL_ABORT");
let v_local = local_schema_version().expect("storage should not fail");
let v_pending: u64 = storage_properties
.pending_schema_version()
diff --git a/test/int/test_ddl.py b/test/int/test_ddl.py
index 6ea87cf68d..84ce4390c4 100644
--- a/test/int/test_ddl.py
+++ b/test/int/test_ddl.py
@@ -2,7 +2,6 @@ import pytest
from conftest import (
PICO_SERVICE_ID,
Cluster,
- ReturnError,
Retriable,
Instance,
TarantoolError,
@@ -12,12 +11,86 @@ from conftest import (
def test_ddl_abort(cluster: Cluster):
- cluster.deploy(instance_count=2)
+ i1, i2, i3 = cluster.deploy(instance_count=3, init_replication_factor=1)
+
+ error_injection = "BLOCK_GOVERNOR_BEFORE_DDL_ABORT"
+ injection_log = f"ERROR INJECTION '{error_injection}'"
+ lc = log_crawler(i3, injection_log)
+
+ # Enable error injection
+ i3.env[f"PICODATA_ERROR_INJECTION_{error_injection}"] = "1"
+ i3.wait_online()
+
+ # Create a conflict to force ddl abort.
+ space_name = "space_name_conflict"
+ i3.eval("box.schema.space.create(...)", space_name)
+
+ # Terminate i3 so that other instances actually partially apply the ddl.
+ i3.terminate()
+
+ # Initiate ddl create space.
+ space_id = 887
+ index_abort = i1.propose_create_space(
+ dict(
+ id=space_id,
+ name=space_name,
+ format=[
+ dict(name="id", type="unsigned", is_nullable=False),
+ ],
+ primary_key=[dict(field="id")],
+ distribution=dict(
+ kind="sharded_implicitly",
+ sharding_key=["id"],
+ sharding_fn="murmur3",
+ tier="default",
+ ),
+ engine="memtx",
+ owner=0,
+ ),
+ wait_index=False,
+ )
+
+ index_prepare = index_abort - 1
+ i1.raft_wait_index(index_prepare)
+ i2.raft_wait_index(index_prepare)
+
+ def get_index_names(i, space_id):
+ return i.eval(
+ """
+ local space_id = ...
+ local res = box.space._pico_index:select({space_id})
+ for i, t in ipairs(res) do
+ res[i] = t.name
+ end
+ return res
+ """,
+ space_id,
+ )
+
+ assert i1.call("box.space._space:get", space_id) is not None
+ assert get_index_names(i1, space_id) == [f"{space_name}_pkey"]
+ assert i2.call("box.space._space:get", space_id) is not None
+ assert get_index_names(i2, space_id) == [f"{space_name}_pkey"]
+
+ # Wake the instance so that governor finds out there's a conflict
+ # and aborts the ddl op.
+ i3.start()
+ i3.wait_online()
+ lc.wait_matched()
+
+ i3.call("pico._inject_error", error_injection, False)
+
+ i1.raft_wait_index(index_abort, timeout=10)
+ i2.raft_wait_index(index_abort, timeout=10)
+ i3.raft_wait_index(index_abort, timeout=10)
- with pytest.raises(ReturnError, match="there is no pending ddl operation"):
- cluster.abort_ddl()
+ def check_space_removed(instance):
+ assert instance.call("box.space._space:get", space_id) is None
+ assert get_index_names(instance, space_id) == []
- # TODO: test manual abort when we have long-running ddls
+ Retriable(timeout=10).call(check_space_removed, i1)
+ Retriable(timeout=10).call(check_space_removed, i2)
+ Retriable(timeout=10).call(check_space_removed, i3)
################################################################################
--
GitLab