|
 |
50dc83 |
From 69ac1fd2da7a57f2f0854412863911959bf71fde Mon Sep 17 00:00:00 2001
|
|
|
50dc83 |
From: Sunny Kumar <sunkumar@redhat.com>
|
|
|
50dc83 |
Date: Tue, 2 Apr 2019 12:38:09 +0530
|
|
|
50dc83 |
Subject: [PATCH 161/169] geo-rep: Fix rename with existing destination with
|
|
|
50dc83 |
same gfid
|
|
|
50dc83 |
|
|
|
50dc83 |
Problem:
|
|
|
50dc83 |
Geo-rep fails to sync the rename properly if destination exists.
|
|
|
50dc83 |
It results in source to be remained on slave causing more number of
|
|
|
50dc83 |
files on slave. Also heavy rename workload like logrotate caused
|
|
|
50dc83 |
lot of ESTALE errors
|
|
|
50dc83 |
|
|
|
50dc83 |
Cause:
|
|
|
50dc83 |
Geo-rep fails to sync rename if destination exists if creation
|
|
|
50dc83 |
of source file also falls into single batch of changelogs being
|
|
|
50dc83 |
processed. This is because, after fixing problematic gfids verifying
|
|
|
50dc83 |
from master, while re-processing original entries, CREATE also was
|
|
|
50dc83 |
re-processed causing more files on slave and rename to be failed.
|
|
|
50dc83 |
|
|
|
50dc83 |
Solution:
|
|
|
50dc83 |
Entries need to be removed from retrial list after fixing
|
|
|
50dc83 |
problematic gfids on slave so that it's not re-created again on slave.
|
|
|
50dc83 |
Also treat ESTALE as EEXIST so that the error is properly handled
|
|
|
50dc83 |
verifying the op on master volume.
|
|
|
50dc83 |
|
|
|
50dc83 |
Backport of:
|
|
|
50dc83 |
> Patch: https://review.gluster.org/22519
|
|
|
50dc83 |
> Change-Id: I50cf289e06b997adddff0552bf2466d9201dd1f9
|
|
|
50dc83 |
> BUG: 1694820
|
|
|
50dc83 |
> Signed-off-by: Kotresh HR <khiremat@redhat.com>
|
|
|
50dc83 |
> Signed-off-by: Sunny Kumar <sunkumar@redhat.com>
|
|
|
50dc83 |
|
|
|
50dc83 |
Change-Id: I50cf289e06b997adddff0552bf2466d9201dd1f9
|
|
|
50dc83 |
fixes: bz#1708121
|
|
|
50dc83 |
Signed-off-by: Kotresh HR <khiremat@redhat.com>
|
|
|
50dc83 |
Signed-off-by: Sunny Kumar <sunkumar@redhat.com>
|
|
|
50dc83 |
Reviewed-on: https://code.engineering.redhat.com/gerrit/172393
|
|
|
50dc83 |
Tested-by: RHGS Build Bot <nigelb@redhat.com>
|
|
|
50dc83 |
Reviewed-by: Atin Mukherjee <amukherj@redhat.com>
|
|
|
50dc83 |
---
|
|
|
50dc83 |
geo-replication/syncdaemon/master.py | 41 +++++++++++++++++++++--
|
|
|
50dc83 |
geo-replication/syncdaemon/resource.py | 10 ++++--
|
|
|
50dc83 |
tests/00-geo-rep/georep-basic-dr-rsync-arbiter.t | 5 +++
|
|
|
50dc83 |
tests/00-geo-rep/georep-basic-dr-rsync.t | 5 +++
|
|
|
50dc83 |
tests/00-geo-rep/georep-basic-dr-tarssh-arbiter.t | 5 +++
|
|
|
50dc83 |
tests/00-geo-rep/georep-basic-dr-tarssh.t | 5 +++
|
|
|
50dc83 |
tests/geo-rep.rc | 36 ++++++++++++++++++++
|
|
|
50dc83 |
7 files changed, 102 insertions(+), 5 deletions(-)
|
|
|
50dc83 |
|
|
|
50dc83 |
diff --git a/geo-replication/syncdaemon/master.py b/geo-replication/syncdaemon/master.py
|
|
|
50dc83 |
index 3da7610..42c86d7 100644
|
|
|
50dc83 |
--- a/geo-replication/syncdaemon/master.py
|
|
|
50dc83 |
+++ b/geo-replication/syncdaemon/master.py
|
|
|
50dc83 |
@@ -65,6 +65,9 @@ def _volinfo_hook_relax_foreign(self):
|
|
|
50dc83 |
def edct(op, **ed):
|
|
|
50dc83 |
dct = {}
|
|
|
50dc83 |
dct['op'] = op
|
|
|
50dc83 |
+ # This is used in automatic gfid conflict resolution.
|
|
|
50dc83 |
+ # When marked True, it's skipped during re-processing.
|
|
|
50dc83 |
+ dct['skip_entry'] = False
|
|
|
50dc83 |
for k in ed:
|
|
|
50dc83 |
if k == 'stat':
|
|
|
50dc83 |
st = ed[k]
|
|
|
50dc83 |
@@ -792,6 +795,7 @@ class GMasterChangelogMixin(GMasterCommon):
|
|
|
50dc83 |
pfx = gauxpfx()
|
|
|
50dc83 |
fix_entry_ops = []
|
|
|
50dc83 |
failures1 = []
|
|
|
50dc83 |
+ remove_gfids = set()
|
|
|
50dc83 |
for failure in failures:
|
|
|
50dc83 |
if failure[2]['name_mismatch']:
|
|
|
50dc83 |
pbname = failure[2]['slave_entry']
|
|
|
50dc83 |
@@ -822,6 +826,18 @@ class GMasterChangelogMixin(GMasterCommon):
|
|
|
50dc83 |
edct('UNLINK',
|
|
|
50dc83 |
gfid=failure[2]['slave_gfid'],
|
|
|
50dc83 |
entry=pbname))
|
|
|
50dc83 |
+ remove_gfids.add(slave_gfid)
|
|
|
50dc83 |
+ if op in ['RENAME']:
|
|
|
50dc83 |
+ # If renamed gfid doesn't exists on master, remove
|
|
|
50dc83 |
+ # rename entry and unlink src on slave
|
|
|
50dc83 |
+ st = lstat(os.path.join(pfx, failure[0]['gfid']))
|
|
|
50dc83 |
+ if isinstance(st, int) and st == ENOENT:
|
|
|
50dc83 |
+ logging.debug("Unlink source %s" % repr(failure))
|
|
|
50dc83 |
+ remove_gfids.add(failure[0]['gfid'])
|
|
|
50dc83 |
+ fix_entry_ops.append(
|
|
|
50dc83 |
+ edct('UNLINK',
|
|
|
50dc83 |
+ gfid=failure[0]['gfid'],
|
|
|
50dc83 |
+ entry=failure[0]['entry']))
|
|
|
50dc83 |
# Takes care of scenarios of hardlinks/renames on master
|
|
|
50dc83 |
elif not isinstance(st, int):
|
|
|
50dc83 |
if matching_disk_gfid(slave_gfid, pbname):
|
|
|
50dc83 |
@@ -831,7 +847,12 @@ class GMasterChangelogMixin(GMasterCommon):
|
|
|
50dc83 |
' Safe to ignore, take out entry',
|
|
|
50dc83 |
retry_count=retry_count,
|
|
|
50dc83 |
entry=repr(failure)))
|
|
|
50dc83 |
- entries.remove(failure[0])
|
|
|
50dc83 |
+ remove_gfids.add(failure[0]['gfid'])
|
|
|
50dc83 |
+ if op == 'RENAME':
|
|
|
50dc83 |
+ fix_entry_ops.append(
|
|
|
50dc83 |
+ edct('UNLINK',
|
|
|
50dc83 |
+ gfid=failure[0]['gfid'],
|
|
|
50dc83 |
+ entry=failure[0]['entry']))
|
|
|
50dc83 |
# The file exists on master but with different name.
|
|
|
50dc83 |
# Probably renamed and got missed during xsync crawl.
|
|
|
50dc83 |
elif failure[2]['slave_isdir']:
|
|
|
50dc83 |
@@ -856,7 +877,10 @@ class GMasterChangelogMixin(GMasterCommon):
|
|
|
50dc83 |
'take out entry',
|
|
|
50dc83 |
retry_count=retry_count,
|
|
|
50dc83 |
entry=repr(failure)))
|
|
|
50dc83 |
- entries.remove(failure[0])
|
|
|
50dc83 |
+ try:
|
|
|
50dc83 |
+ entries.remove(failure[0])
|
|
|
50dc83 |
+ except ValueError:
|
|
|
50dc83 |
+ pass
|
|
|
50dc83 |
else:
|
|
|
50dc83 |
rename_dict = edct('RENAME', gfid=slave_gfid,
|
|
|
50dc83 |
entry=src_entry,
|
|
|
50dc83 |
@@ -896,7 +920,10 @@ class GMasterChangelogMixin(GMasterCommon):
|
|
|
50dc83 |
'ignore, take out entry',
|
|
|
50dc83 |
retry_count=retry_count,
|
|
|
50dc83 |
entry=repr(failure)))
|
|
|
50dc83 |
- entries.remove(failure[0])
|
|
|
50dc83 |
+ try:
|
|
|
50dc83 |
+ entries.remove(failure[0])
|
|
|
50dc83 |
+ except ValueError:
|
|
|
50dc83 |
+ pass
|
|
|
50dc83 |
else:
|
|
|
50dc83 |
logging.info(lf('Fixing ENOENT error in slave. Create '
|
|
|
50dc83 |
'parent directory on slave.',
|
|
|
50dc83 |
@@ -913,6 +940,14 @@ class GMasterChangelogMixin(GMasterCommon):
|
|
|
50dc83 |
edct('MKDIR', gfid=pargfid, entry=dir_entry,
|
|
|
50dc83 |
mode=st.st_mode, uid=st.st_uid, gid=st.st_gid))
|
|
|
50dc83 |
|
|
|
50dc83 |
+ logging.debug("remove_gfids: %s" % repr(remove_gfids))
|
|
|
50dc83 |
+ if remove_gfids:
|
|
|
50dc83 |
+ for e in entries:
|
|
|
50dc83 |
+ if e['op'] in ['MKDIR', 'MKNOD', 'CREATE', 'RENAME'] \
|
|
|
50dc83 |
+ and e['gfid'] in remove_gfids:
|
|
|
50dc83 |
+ logging.debug("Removed entry op from retrial list: entry: %s" % repr(e))
|
|
|
50dc83 |
+ e['skip_entry'] = True
|
|
|
50dc83 |
+
|
|
|
50dc83 |
if fix_entry_ops:
|
|
|
50dc83 |
# Process deletions of entries whose gfids are mismatched
|
|
|
50dc83 |
failures1 = self.slave.server.entry_ops(fix_entry_ops)
|
|
|
50dc83 |
diff --git a/geo-replication/syncdaemon/resource.py b/geo-replication/syncdaemon/resource.py
|
|
|
50dc83 |
index c290d86..f54ccd9 100644
|
|
|
50dc83 |
--- a/geo-replication/syncdaemon/resource.py
|
|
|
50dc83 |
+++ b/geo-replication/syncdaemon/resource.py
|
|
|
50dc83 |
@@ -426,7 +426,7 @@ class Server(object):
|
|
|
50dc83 |
e['stat']['uid'] = uid
|
|
|
50dc83 |
e['stat']['gid'] = gid
|
|
|
50dc83 |
|
|
|
50dc83 |
- if cmd_ret == EEXIST:
|
|
|
50dc83 |
+ if cmd_ret in [EEXIST, ESTALE]:
|
|
|
50dc83 |
if dst:
|
|
|
50dc83 |
en = e['entry1']
|
|
|
50dc83 |
else:
|
|
|
50dc83 |
@@ -510,6 +510,12 @@ class Server(object):
|
|
|
50dc83 |
entry = e['entry']
|
|
|
50dc83 |
uid = 0
|
|
|
50dc83 |
gid = 0
|
|
|
50dc83 |
+
|
|
|
50dc83 |
+ # Skip entry processing if it's marked true during gfid
|
|
|
50dc83 |
+ # conflict resolution
|
|
|
50dc83 |
+ if e['skip_entry']:
|
|
|
50dc83 |
+ continue
|
|
|
50dc83 |
+
|
|
|
50dc83 |
if e.get("stat", {}):
|
|
|
50dc83 |
# Copy UID/GID value and then reset to zero. Copied UID/GID
|
|
|
50dc83 |
# will be used to run chown once entry is created.
|
|
|
50dc83 |
@@ -688,7 +694,7 @@ class Server(object):
|
|
|
50dc83 |
if blob:
|
|
|
50dc83 |
cmd_ret = errno_wrap(Xattr.lsetxattr,
|
|
|
50dc83 |
[pg, 'glusterfs.gfid.newfile', blob],
|
|
|
50dc83 |
- [EEXIST, ENOENT],
|
|
|
50dc83 |
+ [EEXIST, ENOENT, ESTALE],
|
|
|
50dc83 |
[ESTALE, EINVAL, EBUSY])
|
|
|
50dc83 |
collect_failure(e, cmd_ret, uid, gid)
|
|
|
50dc83 |
|
|
|
50dc83 |
diff --git a/tests/00-geo-rep/georep-basic-dr-rsync-arbiter.t b/tests/00-geo-rep/georep-basic-dr-rsync-arbiter.t
|
|
|
50dc83 |
index 67ac167..1a55ed2 100644
|
|
|
50dc83 |
--- a/tests/00-geo-rep/georep-basic-dr-rsync-arbiter.t
|
|
|
50dc83 |
+++ b/tests/00-geo-rep/georep-basic-dr-rsync-arbiter.t
|
|
|
50dc83 |
@@ -203,6 +203,11 @@ EXPECT_WITHIN $GEO_REP_TIMEOUT 0 verify_rsnapshot_data ${slave_mnt}
|
|
|
50dc83 |
TEST gluster volume geo-rep $master $slave config rsync-options "--whole-file"
|
|
|
50dc83 |
TEST "echo sampledata > $master_mnt/rsync_option_test_file"
|
|
|
50dc83 |
|
|
|
50dc83 |
+#rename with existing destination case BUG:1694820
|
|
|
50dc83 |
+TEST create_rename_with_existing_destination ${master_mnt}
|
|
|
50dc83 |
+#verify rename with existing destination case BUG:1694820
|
|
|
50dc83 |
+EXPECT_WITHIN $GEO_REP_TIMEOUT 0 verify_rename_with_existing_destination ${slave_mnt}
|
|
|
50dc83 |
+
|
|
|
50dc83 |
#Verify arequal for whole volume
|
|
|
50dc83 |
EXPECT_WITHIN $GEO_REP_TIMEOUT 0 arequal_checksum ${master_mnt} ${slave_mnt}
|
|
|
50dc83 |
|
|
|
50dc83 |
diff --git a/tests/00-geo-rep/georep-basic-dr-rsync.t b/tests/00-geo-rep/georep-basic-dr-rsync.t
|
|
|
50dc83 |
index 8b64370..d0c0fc9 100644
|
|
|
50dc83 |
--- a/tests/00-geo-rep/georep-basic-dr-rsync.t
|
|
|
50dc83 |
+++ b/tests/00-geo-rep/georep-basic-dr-rsync.t
|
|
|
50dc83 |
@@ -204,6 +204,11 @@ EXPECT_WITHIN $GEO_REP_TIMEOUT 0 verify_rsnapshot_data ${slave_mnt}
|
|
|
50dc83 |
TEST gluster volume geo-rep $master $slave config rsync-options "--whole-file"
|
|
|
50dc83 |
TEST "echo sampledata > $master_mnt/rsync_option_test_file"
|
|
|
50dc83 |
|
|
|
50dc83 |
+#rename with existing destination case BUG:1694820
|
|
|
50dc83 |
+TEST create_rename_with_existing_destination ${master_mnt}
|
|
|
50dc83 |
+#verify rename with existing destination case BUG:1694820
|
|
|
50dc83 |
+EXPECT_WITHIN $GEO_REP_TIMEOUT 0 verify_rename_with_existing_destination ${slave_mnt}
|
|
|
50dc83 |
+
|
|
|
50dc83 |
#Verify arequal for whole volume
|
|
|
50dc83 |
EXPECT_WITHIN $GEO_REP_TIMEOUT 0 arequal_checksum ${master_mnt} ${slave_mnt}
|
|
|
50dc83 |
|
|
|
50dc83 |
diff --git a/tests/00-geo-rep/georep-basic-dr-tarssh-arbiter.t b/tests/00-geo-rep/georep-basic-dr-tarssh-arbiter.t
|
|
|
50dc83 |
index 1726d0b..cb530ad 100644
|
|
|
50dc83 |
--- a/tests/00-geo-rep/georep-basic-dr-tarssh-arbiter.t
|
|
|
50dc83 |
+++ b/tests/00-geo-rep/georep-basic-dr-tarssh-arbiter.t
|
|
|
50dc83 |
@@ -202,6 +202,11 @@ EXPECT_WITHIN $GEO_REP_TIMEOUT 0 verify_symlink_rename_mkdir_data ${slave_mnt}/s
|
|
|
50dc83 |
#rsnapshot usecase
|
|
|
50dc83 |
EXPECT_WITHIN $GEO_REP_TIMEOUT 0 verify_rsnapshot_data ${slave_mnt}
|
|
|
50dc83 |
|
|
|
50dc83 |
+#rename with existing destination case BUG:1694820
|
|
|
50dc83 |
+TEST create_rename_with_existing_destination ${master_mnt}
|
|
|
50dc83 |
+#verify rename with existing destination case BUG:1694820
|
|
|
50dc83 |
+EXPECT_WITHIN $GEO_REP_TIMEOUT 0 verify_rename_with_existing_destination ${slave_mnt}
|
|
|
50dc83 |
+
|
|
|
50dc83 |
#Verify arequal for whole volume
|
|
|
50dc83 |
EXPECT_WITHIN $GEO_REP_TIMEOUT 0 arequal_checksum ${master_mnt} ${slave_mnt}
|
|
|
50dc83 |
|
|
|
50dc83 |
diff --git a/tests/00-geo-rep/georep-basic-dr-tarssh.t b/tests/00-geo-rep/georep-basic-dr-tarssh.t
|
|
|
50dc83 |
index c5d16ac..9e2f613 100644
|
|
|
50dc83 |
--- a/tests/00-geo-rep/georep-basic-dr-tarssh.t
|
|
|
50dc83 |
+++ b/tests/00-geo-rep/georep-basic-dr-tarssh.t
|
|
|
50dc83 |
@@ -202,6 +202,11 @@ EXPECT_WITHIN $GEO_REP_TIMEOUT 0 verify_hardlink_rename_data ${slave_mnt}
|
|
|
50dc83 |
#rsnapshot usecase
|
|
|
50dc83 |
EXPECT_WITHIN $GEO_REP_TIMEOUT 0 verify_rsnapshot_data ${slave_mnt}
|
|
|
50dc83 |
|
|
|
50dc83 |
+#rename with existing destination case BUG:1694820
|
|
|
50dc83 |
+TEST create_rename_with_existing_destination ${master_mnt}
|
|
|
50dc83 |
+#verify rename with existing destination case BUG:1694820
|
|
|
50dc83 |
+EXPECT_WITHIN $GEO_REP_TIMEOUT 0 verify_rename_with_existing_destination ${slave_mnt}
|
|
|
50dc83 |
+
|
|
|
50dc83 |
#Verify arequal for whole volume
|
|
|
50dc83 |
EXPECT_WITHIN $GEO_REP_TIMEOUT 0 arequal_checksum ${master_mnt} ${slave_mnt}
|
|
|
50dc83 |
|
|
|
50dc83 |
diff --git a/tests/geo-rep.rc b/tests/geo-rep.rc
|
|
|
50dc83 |
index d723129..e357ba8 100644
|
|
|
50dc83 |
--- a/tests/geo-rep.rc
|
|
|
50dc83 |
+++ b/tests/geo-rep.rc
|
|
|
50dc83 |
@@ -403,3 +403,39 @@ function check_slave_read_only()
|
|
|
50dc83 |
gluster volume info $1 | grep 'features.read-only: on'
|
|
|
50dc83 |
echo $?
|
|
|
50dc83 |
}
|
|
|
50dc83 |
+
|
|
|
50dc83 |
+function create_rename_with_existing_destination()
|
|
|
50dc83 |
+{
|
|
|
50dc83 |
+ dir=$1/rename_with_existing_destination
|
|
|
50dc83 |
+ mkdir $dir
|
|
|
50dc83 |
+ for i in {1..5}
|
|
|
50dc83 |
+ do
|
|
|
50dc83 |
+ echo "Data_set$i" > $dir/data_set$i
|
|
|
50dc83 |
+ mv $dir/data_set$i $dir/data_set -f
|
|
|
50dc83 |
+ done
|
|
|
50dc83 |
+}
|
|
|
50dc83 |
+
|
|
|
50dc83 |
+function verify_rename_with_existing_destination()
|
|
|
50dc83 |
+{
|
|
|
50dc83 |
+ dir=$1/rename_with_existing_destination
|
|
|
50dc83 |
+
|
|
|
50dc83 |
+ if [ ! -d $dir ]; then
|
|
|
50dc83 |
+ echo 1
|
|
|
50dc83 |
+ elif [ ! -f $dir/data_set ]; then
|
|
|
50dc83 |
+ echo 2
|
|
|
50dc83 |
+ elif [ -f $dir/data_set1 ]; then
|
|
|
50dc83 |
+ echo 3
|
|
|
50dc83 |
+ elif [ -f $dir/data_set2 ]; then
|
|
|
50dc83 |
+ echo 4
|
|
|
50dc83 |
+ elif [ -f $dir/data_set3 ]; then
|
|
|
50dc83 |
+ echo 5
|
|
|
50dc83 |
+ elif [ -f $dir/data_set4 ]; then
|
|
|
50dc83 |
+ echo 6
|
|
|
50dc83 |
+ elif [ -f $dir/data_set5 ]; then
|
|
|
50dc83 |
+ echo 7
|
|
|
50dc83 |
+ elif test "XData_set5" != "X$(cat $dir/data_set)"; then
|
|
|
50dc83 |
+ echo 8
|
|
|
50dc83 |
+ else
|
|
|
50dc83 |
+ echo 0
|
|
|
50dc83 |
+ fi
|
|
|
50dc83 |
+}
|
|
|
50dc83 |
--
|
|
|
50dc83 |
1.8.3.1
|
|
|
50dc83 |
|