From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from smtp38.i.mail.ru (smtp38.i.mail.ru [94.100.177.98]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by dev.tarantool.org (Postfix) with ESMTPS id 5922F469719 for ; Fri, 13 Nov 2020 15:42:23 +0300 (MSK) References: <20200330144823.GA55948@pony.bronevichok.ru> <20200515161727.jtlqplcdkbbcsgh2@tkn_work_nb> <20200528093934.GA19447@pony.bronevichok.ru> <20200602111351.fctlh3aly3euuo4m@tkn_work_nb> <20200610112919.GA78626@pony.bronevichok.ru> <20201110095546.GA3880@pony.bronevichok.ru> From: Leonid Vasiliev Message-ID: <2817d4c9-0ce6-c6b0-75d3-af803d68bb89@tarantool.org> Date: Fri, 13 Nov 2020 15:41:43 +0300 MIME-Version: 1.0 In-Reply-To: <20201110095546.GA3880@pony.bronevichok.ru> Content-Type: text/plain; charset=utf-8; format=flowed Content-Language: en-US Content-Transfer-Encoding: 7bit Subject: Re: [Tarantool-patches] [test-run] Add options for upgrade testing List-Id: Tarantool development patches List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Sergey Bronnikov , Alexander Turenko Cc: tarantool-patches@dev.tarantool.org Hi! Thank your for the patch. LGTM. To add per line comments, I copied "diff" and marked my comments with "Comment!" in that. See some minor comments below: Option '--snapshot-version' specifies a path to snapshot that will be loaded in Tarantool before testing. Option '--disable-schema-upgrade' allows to skip execution of Tarantool upgrade script using error injection mechanism. Part of: https://github.com/tarantool/tarantool/issues/4801 --- lib/app_server.py | 30 ++++++++++++++++++++++++++++-- lib/options.py | 26 ++++++++++++++++++++++++++ lib/server.py | 32 ++++++++++++++++++++++++++++---- lib/tarantool_server.py | 33 ++++++++++++++++++++++++++++----- lib/utils.py | 30 ++++++++++++++++++++++++++++++ 5 files changed, 140 insertions(+), 11 deletions(-) diff --git a/lib/app_server.py b/lib/app_server.py index 2cb8a87..efe9096 100644 --- a/lib/app_server.py +++ b/lib/app_server.py @@ -1,12 +1,15 @@ import errno import glob import os +import re import shutil +import shlex +import subprocess import sys from gevent.subprocess import Popen, PIPE -from lib.colorer import color_log +from lib.colorer import color_log, color_stdout from lib.preprocessor import TestState from lib.server import Server from lib.tarantool_server import Test @@ -73,6 +76,10 @@ class AppServer(Server): self.binary = TarantoolServer.binary self.use_unix_sockets_iproto = ini['use_unix_sockets_iproto'] + if self.disable_schema_upgrade and not self.test_debug(): + color_stdout("Can't disable schema upgrade on release build\n", + schema='error') + @property def logfile(self): # remove suite name using basename @@ -86,7 +93,12 @@ class AppServer(Server): return os.path.join(self.vardir, file_name) def prepare_args(self, args=[]): - return [os.path.join(os.getcwd(), self.current_test.name)] + args + cli_args = [os.path.join(os.getcwd(), self.current_test.name)] + args + if self.disable_schema_upgrade: + cli_args = [self.binary, '-e', + self.DISABLE_AUTO_UPGRADE] + cli_args + + return cli_args def deploy(self, vardir=None, silent=True, need_init=True): self.vardir = vardir @@ -170,3 +182,17 @@ class AppServer(Server): test_suite.ini)) test_suite.tests = tests + + def test_option_get(self, option_list_str, silent=False): + args = [self.binary] + shlex.split(option_list_str) + if not silent: + print " ".join([os.path.basename(self.binary)] + args[1:]) + output = subprocess.Popen(args, + stdout=subprocess.PIPE, + stderr=subprocess.STDOUT).stdout.read() + return output + + def test_debug(self): + if re.findall(r"-Debug", self.test_option_get("-V", True), re.I): + return True + return False diff --git a/lib/options.py b/lib/options.py index 47bbc0f..0a318ef 100644 --- a/lib/options.py +++ b/lib/options.py @@ -209,6 +209,20 @@ class Options: help="""Update or create file with reference output (.result). Default: false.""") + parser.add_argument( + "--snapshot-path", + dest='snapshot_path', + default=None, + help="""Path to a Tarantool snapshot that will be + loaded before testing.""") + + parser.add_argument( + "--disable-schema-upgrade", + dest='disable_schema_upgrade', + action="store_true", + default=False, + help="""Disable schema upgrade on testing with snapshots.""") + # XXX: We can use parser.parse_intermixed_args() on # Python 3.7 to understand commands like # ./test-run.py foo --exclude bar baz @@ -227,5 +241,17 @@ class Options: color_stdout(format_str.format(op1, op2), schema='error') check_error = True + if getattr(self, 'disable_schema_upgrade', '') and \ + not (self, 'snapshot_path', ''): + color_stdout("option --disable-schema-upgrade \ + depends on --snapshot-version\n", schema='error') + check_error = True + + if getattr(self, 'snapshot_path', ''): + snapshot_path = getattr(self, 'snapshot_path', '') + if not os.path.exists(snapshot_path): + color_stdout("path {} not exist\n", snapshot_path, schema='error') More than 80 characters per line. + check_error = True + if check_error: exit(-1) diff --git a/lib/server.py b/lib/server.py index 321eac7..b994248 100644 --- a/lib/server.py +++ b/lib/server.py @@ -1,6 +1,7 @@ import glob import os import shutil +import os.path Comment! Unneeded. "os" has been imported previously. from itertools import product from lib.server_mixins import ValgrindMixin from lib.server_mixins import GdbMixin @@ -8,7 +9,8 @@ from lib.server_mixins import GdbServerMixin from lib.server_mixins import LLdbMixin from lib.server_mixins import StraceMixin from lib.server_mixins import LuacovMixin -from lib.colorer import color_stdout +from lib.colorer import color_stdout, color_log +from lib.options import Options from lib.utils import print_tail_n @@ -24,6 +26,9 @@ class Server(object): DEFAULT_INSPECTOR = 0 TEST_RUN_DIR = os.path.abspath(os.path.join(os.path.dirname(__file__), "..")) + DISABLE_AUTO_UPGRADE = "require('fiber').sleep(0) \ + assert(box.error.injection.set('ERRINJ_AUTO_UPGRADE', true) == 'ok', \ + 'no such errinj')" Comment! If I understand correctly, Turenko wanted to mark this with a comment about https://github.com/tarantool/tarantool/issues/4983 @property def vardir(self): @@ -73,8 +78,7 @@ class Server(object): core = ini['core'].lower().strip() cls.mdlname = "lib.{0}_server".format(core.replace(' ', '_')) cls.clsname = "{0}Server".format(core.title().replace(' ', '')) - corecls = __import__(cls.mdlname, - fromlist=cls.clsname).__dict__[cls.clsname] + corecls = __import__(cls.mdlname, fromlist=cls.clsname).__dict__[cls.clsname] Comment! Unneeded changes and now it's more than 80 symbols per line. return corecls.__new__(corecls, ini, *args, **kwargs) def __init__(self, ini, test_suite=None): @@ -84,6 +88,9 @@ class Server(object): self.inspector_port = int(ini.get( 'inspector_port', self.DEFAULT_INSPECTOR )) + self.testdir = os.path.abspath(os.curdir) + self.disable_schema_upgrade = None + self.snapshot_path = Options().args.snapshot_path # filled in {Test,AppTest,LuaTest,PythonTest}.execute() # or passed through execfile() for PythonTest (see @@ -94,6 +101,15 @@ class Server(object): # default servers running in TestSuite.run_all() self.test_suite = test_suite + self.disable_schema_upgrade = Options().args.disable_schema_upgrade + if self.snapshot_path: + self.snapshot_path = os.path.abspath(self.snapshot_path) + if os.path.exists(self.snapshot_path): + self.snapshot_basename = os.path.basename(self.snapshot_path) + else: + color_stdout("\nSnapshot {} have been not found\n".format(self.snapshot_path), + schema='error') Comment! More than 80 characters per line. But I think it's ok here. + def prepare_args(self, args=[]): return args @@ -110,7 +126,15 @@ class Server(object): os.remove(f) def install(self, binary=None, vardir=None, mem=None, silent=True): - pass + if self.snapshot_basename: + (instance_name, _) = os.path.splitext(os.path.basename(self.script)) + instance_dir = os.path.join(self.vardir, instance_name) + if not os.path.exists(instance_dir): + os.mkdir(instance_dir) + snapshot_dest = os.path.join(instance_dir, "00000000000000000000.snap") Comment! More than 80 characters per line. + color_log("Copying snapshot {} to {}\n".format( + self.snapshot_path, snapshot_dest)) + shutil.copy(self.snapshot_path, snapshot_dest) def init(self): pass diff --git a/lib/tarantool_server.py b/lib/tarantool_server.py index fd102b9..4f4c6b4 100644 --- a/lib/tarantool_server.py +++ b/lib/tarantool_server.py @@ -27,7 +27,7 @@ from lib.colorer import color_stdout, color_log from lib.preprocessor import TestState from lib.server import Server from lib.test import Test -from lib.utils import find_port +from lib.utils import find_port, extract_schema_from_snapshot from lib.utils import format_process from lib.utils import signame from lib.utils import warn_unix_socket @@ -609,7 +609,6 @@ class TarantoolServer(Server): } ini.update(_ini) Server.__init__(self, ini, test_suite) - self.testdir = os.path.abspath(os.curdir) self.sourcedir = os.path.abspath(os.path.join(os.path.basename( sys.argv[0]), "..", "..")) self.name = "default" @@ -641,6 +640,10 @@ class TarantoolServer(Server): if 'test_run_current_test' in caller_globals.keys(): self.current_test = caller_globals['test_run_current_test'] + if self.disable_schema_upgrade and not self.test_debug(): + color_stdout("Can't disable schema upgrade on release build\n", + schema='error') + def __del__(self): self.stop() @@ -745,6 +748,9 @@ class TarantoolServer(Server): path = os.path.join(self.vardir, self.name + '.control') warn_unix_socket(path) + super(TarantoolServer, self).install(self.binary, + self.vardir, None, silent=True) + def deploy(self, silent=True, **kwargs): self.install(silent) self.start(silent=silent, **kwargs) @@ -776,7 +782,13 @@ class TarantoolServer(Server): self.vardir) def prepare_args(self, args=[]): - return [self.ctl_path, 'start', os.path.basename(self.script)] + args + cli_args = [self.ctl_path, 'start', + os.path.basename(self.script)] + args + if self.disable_schema_upgrade: + cli_args = [self.binary, '-e', + self.DISABLE_AUTO_UPGRADE] + cli_args + + return cli_args def pretest_clean(self): # Don't delete snap and logs for 'default' tarantool server @@ -861,6 +873,18 @@ class TarantoolServer(Server): self.admin = CON_SWITCH[self.tests_type]('localhost', port) self.status = 'started' + if not self.ctl_path: + self.ctl_path = self.find_exe(self.builddir) + if self.disable_schema_upgrade: + version = extract_schema_from_snapshot(self.ctl_path, + self.builddir, self.snapshot_path) Comment! More than 80 symbols per line. + current_ver = yaml.safe_load(self.admin.execute( + 'box.space._schema:get{"version"}')) + if version == current_ver: + raise_msg = 'Versions are inequal ({} vs {})'.format( + version, current_ver) + raise Exception(raise_msg) + def crash_detect(self): if self.crash_expected: return @@ -1066,7 +1090,6 @@ class TarantoolServer(Server): if not silent: print " ".join([os.path.basename(self.binary)] + args[1:]) output = subprocess.Popen(args, - cwd=self.vardir, stdout=subprocess.PIPE, stderr=subprocess.STDOUT).stdout.read() return output @@ -1126,7 +1149,7 @@ class TarantoolServer(Server): def get_param(self, param=None): if param is not None: return yaml.safe_load(self.admin("box.info." + param, - silent=True))[0] + silent=True))[0] return yaml.safe_load(self.admin("box.info", silent=True)) def get_lsn(self, node_id): diff --git a/lib/utils.py b/lib/utils.py index cc2f6b7..7401d5c 100644 --- a/lib/utils.py +++ b/lib/utils.py @@ -3,8 +3,10 @@ import sys import six import collections import signal +import subprocess import random import fcntl +import json import difflib import time from gevent import socket @@ -264,3 +266,31 @@ def just_and_trim(src, width): if len(src) > width: return src[:width - 1] + '>' return src.ljust(width) + + +def extract_schema_from_snapshot(ctl_path, builddir, snapshot_path): + """ + Extract schema version from snapshot, example of record: + {"HEADER":{"lsn":2,"type":"INSERT","timestamp":1584694286.0031}, + "BODY":{"space_id":272,"tuple":["version",2,3,1]}} + """ Comment! If my internal python interpreter is working fine, the return value is a list: "['version', 2, 3, 1]". Typically version is a string like:"2.3.1". I don't mind, but specify it in description. + SCHEMA_SPACE_ID = 272 + + ctl_args = [ctl_path, 'cat', snapshot_path, + '--format=json', '--show-system'] + proc = subprocess.Popen(ctl_args, stdout=subprocess.PIPE) + version = None + while True: + line = proc.stdout.readline() + if not line: + break + json_line = json.loads(line.strip()) + query_type = json_line["HEADER"]["type"] + space_id = json_line["BODY"]["space_id"] + if query_type == "INSERT" or query_type == "REPLACE" and space_id == SCHEMA_SPACE_ID: Comment! More than 80 characters per line. + query_tuple = json_line['BODY']['tuple'] + if query_tuple[0] == 'version': + version = query_tuple + break + + return version -- 2.7.4