Skip to content

Commit

Permalink
The yb_backup.py script must support python2 and python3.
Browse files Browse the repository at this point in the history 
Summary:
- Fixed compatibility issues between python2 / python3 in the yb_backup.py script.
- Minor code optimizations in the yb_backup.py script - in create_and_upload_metadata_files().
- Added temporary folder deleting in YBBackupTest.* tests.

Test Plan: ybd --cxx-test yb-backup-test_ent

Reviewers: bogdan, mikhail

Reviewed By: mikhail

Subscribers: jenkins-bot, yql

Differential Revision: https://phabricator.dev.yugabyte.com/D8911
  • Loading branch information
@OlegLoginov
OlegLoginov committed Jul 17, 2020
1 parent 6f51933 commit 97975cc
Show file tree
Hide file tree
Showing 4 changed files with 69 additions and 41 deletions.
35 changes: 24 additions & 11 deletions ent/src/yb/tools/yb-backup-test_ent.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@
#include "yb/client/ql-dml-test-base.h"
#include "yb/gutil/strings/split.h"
#include "yb/util/jsonreader.h"
#include "yb/util/random_util.h"
#include "yb/util/subprocess.h"

using std::unique_ptr;
Expand All @@ -36,10 +37,27 @@ class YBBackupTest : public pgwrapper::PgCommandTestBase {
ASSERT_OK(CreateClient());
}

void DoBeforeTearDown() override {
if (!tmp_dir_.empty()) {
LOG(INFO) << "Deleting temporary folder: " << tmp_dir_;
ASSERT_OK(Env::Default()->DeleteRecursively(tmp_dir_));
}
}

string GetTempDir(const string& subdir) {
if (tmp_dir_.empty()) {
EXPECT_OK(Env::Default()->GetTestDirectory(&tmp_dir_));
tmp_dir_ = JoinPathSegments(
tmp_dir_, string(CURRENT_TEST_CASE_NAME()) + '_' + RandomHumanReadableString(8));
}

return JoinPathSegments(tmp_dir_, subdir);
}

Status RunBackupCommand(const vector<string>& args) {
const HostPort pg_hp = cluster_->pgsql_hostport(0);
std::stringstream command;
command << "python2 " << GetToolPath("../../../managed/devops/bin", "yb_backup.py")
command << "python3 " << GetToolPath("../../../managed/devops/bin", "yb_backup.py")
<< " --masters " << cluster_->GetMasterAddresses()
<< " --remote_yb_admin_binary=" << GetToolPath("yb-admin")
<< " --remote_ysql_dump_binary=" << GetPgToolPath("ysql_dump")
Expand All @@ -50,7 +68,7 @@ class YBBackupTest : public pgwrapper::PgCommandTestBase {
<< " --no_ssh"
<< " --no_auto_name";
#if defined(__APPLE__)
command << " --mac";
command << " --mac" << " --verbose";
#endif // defined(__APPLE__)
string backup_cmd;
for (const auto& a : args) {
Expand Down Expand Up @@ -94,16 +112,15 @@ class YBBackupTest : public pgwrapper::PgCommandTestBase {
}

client::TableHandle table_;
string tmp_dir_;
};

TEST_F(YBBackupTest, YB_DISABLE_TEST_IN_SANITIZERS(TestYCQLKeyspaceBackup)) {
client::KeyValueTableTest::CreateTable(
client::Transactional::kFalse, CalcNumTablets(3), client_.get(), &table_);
const string& keyspace = table_.name().namespace_name();

string tmp_dir;
ASSERT_OK(Env::Default()->GetTestDirectory(&tmp_dir));
const auto backup_dir = JoinPathSegments(tmp_dir, "backup");
const string backup_dir = GetTempDir("backup");

ASSERT_OK(RunBackupCommand(
{"--backup_location", backup_dir, "--keyspace", keyspace, "create"}));
Expand All @@ -126,9 +143,7 @@ TEST_F(YBBackupTest, YB_DISABLE_TEST_IN_SANITIZERS(TestYSQLKeyspaceBackup)) {
)#"
));

string tmp_dir;
ASSERT_OK(Env::Default()->GetTestDirectory(&tmp_dir));
const auto backup_dir = JoinPathSegments(tmp_dir, "backup");
const string backup_dir = GetTempDir("backup");

// There is no YCQL keyspace 'yugabyte'.
ASSERT_NOK(RunBackupCommand(
Expand Down Expand Up @@ -233,9 +248,7 @@ TEST_F(YBBackupTest, YB_DISABLE_TEST_IN_SANITIZERS(TestYSQLRestoreBackupToNewKey

ASSERT_NO_FATALS(InsertOneRow("INSERT INTO vendors (v_code, v_name) VALUES (100, 'foo')"));

string tmp_dir;
ASSERT_OK(Env::Default()->GetTestDirectory(&tmp_dir));
const auto backup_dir = JoinPathSegments(tmp_dir, "backup");
const string backup_dir = GetTempDir("backup");

ASSERT_OK(RunBackupCommand(
{"--backup_location", backup_dir, "--keyspace", "ysql.yugabyte", "create"}));
Expand Down
73 changes: 43 additions & 30 deletions managed/devops/bin/yb_backup.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,6 @@

import os
import re
from six import iteritems

TABLET_UUID_LEN = 32
UUID_RE_STR = '[0-9a-f-]{32,36}'
Expand Down Expand Up @@ -253,7 +252,7 @@ def check_uuid(uuid_str):


def random_string(length):
return ''.join(random.choice(string.lowercase) for i in range(length))
return ''.join(random.choice(string.ascii_lowercase) for i in range(length))


def strip_dir(dir_path):
Expand Down Expand Up @@ -523,6 +522,8 @@ def run_program(self, args, num_retry=1, timeout=10, env=None, **kwargs):
env = os.environ.copy()
subprocess_result = subprocess.check_output(args, stderr=subprocess.STDOUT,
env=env, **kwargs)
if not isinstance(subprocess_result, str):
subprocess_result = subprocess_result.decode('utf-8')

if self.args.verbose:
logging.info(
Expand Down Expand Up @@ -1109,8 +1110,9 @@ def find_local_data_dirs(self, tserver_ip):
ip = args[i + 1]

if ip == tserver_ip:
logging.info("Found data directories on server {}: {}".format(ip, fs_data_dirs))
return fs_data_dirs
raise BackupException("Unable find find data directories for {}".format(tserver_ip))
raise BackupException("Unable find data directories on server {}".format(tserver_ip))

def generate_snapshot_dirs(self, data_dir_by_tserver, snapshot_id,
tablets_by_tserver_ip, table_ids):
Expand All @@ -1128,7 +1130,8 @@ def generate_snapshot_dirs(self, data_dir_by_tserver, snapshot_id,
tserver_ip_to_tablet_id_to_snapshot_dirs = {}
deleted_tablets_by_tserver_ip = {}

for tserver_ip, tablets in tablets_by_tserver_ip.iteritems():
for tserver_ip in tablets_by_tserver_ip:
tablets = tablets_by_tserver_ip[tserver_ip]
tablet_dirs = []
data_dirs = data_dir_by_tserver[tserver_ip]
deleted_tablets = deleted_tablets_by_tserver_ip.setdefault(tserver_ip, set())
Expand Down Expand Up @@ -1254,8 +1257,9 @@ def rearrange_snapshot_dirs(
directories for that tablet id that we found.
"""
tserver_ip_to_tablet_id_to_snapshot_dirs = {}
for (data_dir, snapshot_id_unused, tserver_ip), snapshot_dirs in \
iteritems(find_snapshot_dir_results):
for key in find_snapshot_dir_results:
(data_dir, snapshot_id_unused, tserver_ip) = key
snapshot_dirs = find_snapshot_dir_results[key]
assert snapshot_id_unused == snapshot_id
tablet_id_to_snapshot_dirs =\
tserver_ip_to_tablet_id_to_snapshot_dirs.setdefault(tserver_ip, {})
Expand Down Expand Up @@ -1411,10 +1415,11 @@ def prepare_cloud_ssh_cmds(
:param snapshot_metadata: In case of downloading files from cloud to restore a backup,
this is the snapshot metadata stored in cloud for the backup.
"""
for tserver_ip, tablet_id_to_snapshot_dirs in \
iteritems(tserver_ip_to_tablet_id_to_snapshot_dirs):
for tserver_ip in tserver_ip_to_tablet_id_to_snapshot_dirs:
tablet_id_to_snapshot_dirs = tserver_ip_to_tablet_id_to_snapshot_dirs[tserver_ip]
tablet_ids_with_data_dirs = set()
for tablet_id, snapshot_dirs in iteritems(tablet_id_to_snapshot_dirs):
for tablet_id in tablet_id_to_snapshot_dirs:
snapshot_dirs = tablet_id_to_snapshot_dirs[tablet_id]
if len(snapshot_dirs) > 1:
raise BackupException(
('Found multiple snapshot directories on tserver {} for snapshot id '
Expand Down Expand Up @@ -1527,14 +1532,16 @@ def create_and_upload_metadata_files(self, snapshot_filepath):
else:
self.create_remote_tmp_dir(self.get_leader_master_ip())

if self.is_ysql_keyspace():
is_ysql = self.is_ysql_keyspace()
if is_ysql:
sql_dump_path = os.path.join(self.get_tmp_dir(), SQL_DUMP_FILE_NAME)
db_name = keyspace_name(self.args.keyspace[0])
start_version = self.get_ysql_catalog_version()

stored_keyspaces = self.args.keyspace
stored_tables = self.args.table
num_retry = CREATE_METAFILES_MAX_RETRIES
start_version = self.get_ysql_catalog_version() if self.is_ysql_keyspace() else ''

while num_retry > 0:
num_retry = num_retry - 1

Expand All @@ -1547,23 +1554,25 @@ def create_and_upload_metadata_files(self, snapshot_filepath):
logging.info("Snapshot %s will be deleted at exit...", snapshot_id)
atexit.register(self.delete_created_snapshot, snapshot_id)

if self.is_ysql_keyspace():
if is_ysql:
logging.info("Creating ysql dump for DB '{}' to {}".format(db_name, sql_dump_path))
self.run_ysql_dump(['--include-yb-metadata', '--serializable-deferrable',
'--create', '--schema-only',
'--dbname=' + db_name, '--file=' + sql_dump_path])

final_version = self.get_ysql_catalog_version() if self.is_ysql_keyspace() else ''
logging.info('Catalog versions: {} - {}'.format(start_version, final_version))
if final_version == start_version:
break # Ok. No table schema changes during meta data creating.
else:
# wait_for_snapshot() can update the variables - restore them back.
self.args.keyspace = stored_keyspaces
self.args.table = stored_tables
final_version = self.get_ysql_catalog_version()
logging.info('Catalog versions: {} - {}'.format(start_version, final_version))
if final_version == start_version:
break # Ok. No table schema changes during meta data creating.
else:
# wait_for_snapshot() can update the variables - restore them back.
self.args.keyspace = stored_keyspaces
self.args.table = stored_tables

start_version = final_version
logging.info('Retry creating metafiles ({} retries left)'.format(num_retry))
start_version = final_version
logging.info('Retry creating metafiles ({} retries left)'.format(num_retry))
else:
break # Ok. No need to retry for YCQL.

if num_retry == 0:
raise BackupException("Couldn't create metafiles due to catalog changes")
Expand All @@ -1574,7 +1583,7 @@ def create_and_upload_metadata_files(self, snapshot_filepath):
self.upload_metadata_and_checksum(metadata_path,
os.path.join(snapshot_filepath, METADATA_FILE_NAME))

if self.is_ysql_keyspace():
if is_ysql:
self.upload_metadata_and_checksum(sql_dump_path,
os.path.join(snapshot_filepath, SQL_DUMP_FILE_NAME))

Expand Down Expand Up @@ -1779,7 +1788,7 @@ def find_tablet_replicas(self, snapshot_metadata):
"""

tablets_by_tserver_ip = {}
for new_id, old_id in snapshot_metadata['tablet'].iteritems():
for new_id in snapshot_metadata['tablet']:
output = self.run_yb_admin(['list_tablet_servers', new_id])
for line in output.splitlines():
if LEADING_UUID_RE.match(line):
Expand All @@ -1798,7 +1807,8 @@ def identify_new_tablet_replicas(self, tablets_by_tserver_ip_old, tablets_by_tse
tablets_by_tserver_union = copy.deepcopy(tablets_by_tserver_ip_old)
tablets_by_tserver_delta = {}

for ip, tablets in tablets_by_tserver_ip_new.iteritems():
for ip in tablets_by_tserver_ip_new:
tablets = tablets_by_tserver_ip_new[ip]
if ip in tablets_by_tserver_ip_old:
if not (tablets_by_tserver_ip_old[ip] >= tablets):
tablets_by_tserver_union[ip].update(tablets)
Expand All @@ -1824,7 +1834,8 @@ def download_snapshot_directories(self, snapshot_meta, tablets_by_tserver_to_dow
data_dir_by_tserver, snapshot_id, tablets_by_tserver_to_download, table_ids)

# Remove deleted tablets from the list of planned to be downloaded tablets.
for tserver_ip, deleted_tablets in tserver_to_deleted_tablets.iteritems():
for tserver_ip in tserver_to_deleted_tablets:
deleted_tablets = tserver_to_deleted_tablets[tserver_ip]
tablets_by_tserver_to_download[tserver_ip] -= deleted_tablets

parallel_downloads = SequencedParallelCmd(self.run_ssh_cmd)
Expand All @@ -1836,9 +1847,10 @@ def download_snapshot_directories(self, snapshot_meta, tablets_by_tserver_to_dow
# Run a sequence of steps for each tablet, handling different tablets in parallel.
results = parallel_downloads.run(pool)

for k, v in results.iteritems():
if v.strip() != 'correct':
raise BackupException('Check-sum for "{}" is {}'.format(k, v.strip()))
for k in results:
v = results[k].strip()
if v != 'correct':
raise BackupException('Check-sum for "{}" is {}'.format(k, v))

return tserver_to_deleted_tablets

Expand Down Expand Up @@ -1884,7 +1896,8 @@ def restore_table(self):
snapshot_metadata, tablets_by_tserver_to_download, snapshot_id, table_ids)

# Remove deleted tablets from the list of all tablets.
for tserver_ip, deleted_tablets in tserver_to_deleted_tablets.iteritems():
for tserver_ip in tserver_to_deleted_tablets:
deleted_tablets = tserver_to_deleted_tablets[tserver_ip]
all_tablets_by_tserver[tserver_ip] -= deleted_tablets

tablets_by_tserver_new = self.find_tablet_replicas(snapshot_metadata)
Expand Down
1 change: 1 addition & 0 deletions requirements.txt
View file
Original file line number Diff line number Diff line change
Expand Up @@ -4,3 +4,4 @@ yugabyte_pycommon
compiledb
psutil
distro
boto
1 change: 1 addition & 0 deletions requirements_frozen.txt
View file
Original file line number Diff line number Diff line change
Expand Up @@ -7,3 +7,4 @@ six==1.12.0
yugabyte-pycommon==1.9.3
psutil==5.5.1
distro==1.5.0
boto==2.49.0

0 comments on commit 97975cc

Please sign in to comment.