Update share replicas after promotion in proper order
Update replica that becomes active after promotion only as last to avoid concurrency in share replica states when we can have 2 active share replicas in some narrow timeframe. Change-Id: Idf01daf31172a21dbb6595044c11980867346d13 Closes-Bug: #1664201
This commit is contained in:
parent
00adc593fb
commit
f7783776b5
@ -1996,19 +1996,26 @@ class ShareManager(manager.SchedulerDependentManager):
|
|||||||
{'status': constants.STATUS_ERROR})
|
{'status': constants.STATUS_ERROR})
|
||||||
|
|
||||||
if not updated_replica_list:
|
if not updated_replica_list:
|
||||||
self.db.share_replica_update(
|
|
||||||
context, share_replica['id'],
|
|
||||||
{'status': constants.STATUS_AVAILABLE,
|
|
||||||
'replica_state': constants.REPLICA_STATE_ACTIVE,
|
|
||||||
'cast_rules_to_readonly': False})
|
|
||||||
|
|
||||||
self.db.share_replica_update(
|
self.db.share_replica_update(
|
||||||
context, old_active_replica['id'],
|
context, old_active_replica['id'],
|
||||||
{'replica_state': constants.REPLICA_STATE_OUT_OF_SYNC,
|
{'replica_state': constants.REPLICA_STATE_OUT_OF_SYNC,
|
||||||
'cast_rules_to_readonly':
|
'cast_rules_to_readonly':
|
||||||
ensure_old_active_replica_to_readonly})
|
ensure_old_active_replica_to_readonly})
|
||||||
|
self.db.share_replica_update(
|
||||||
|
context, share_replica['id'],
|
||||||
|
{'status': constants.STATUS_AVAILABLE,
|
||||||
|
'replica_state': constants.REPLICA_STATE_ACTIVE,
|
||||||
|
'cast_rules_to_readonly': False})
|
||||||
else:
|
else:
|
||||||
|
while updated_replica_list:
|
||||||
|
# NOTE(vponomaryov): update 'active' replica last.
|
||||||
for updated_replica in updated_replica_list:
|
for updated_replica in updated_replica_list:
|
||||||
|
if (updated_replica['id'] == share_replica['id'] and
|
||||||
|
len(updated_replica_list) > 1):
|
||||||
|
continue
|
||||||
|
updated_replica_list.remove(updated_replica)
|
||||||
|
break
|
||||||
|
|
||||||
updated_export_locs = updated_replica.get(
|
updated_export_locs = updated_replica.get(
|
||||||
'export_locations')
|
'export_locations')
|
||||||
if(updated_export_locs is not None
|
if(updated_export_locs is not None
|
||||||
|
@ -1148,7 +1148,10 @@ class ShareManagerTestCase(test.TestCase):
|
|||||||
active_replica = fake_replica(
|
active_replica = fake_replica(
|
||||||
id='current_active_replica',
|
id='current_active_replica',
|
||||||
replica_state=constants.REPLICA_STATE_ACTIVE)
|
replica_state=constants.REPLICA_STATE_ACTIVE)
|
||||||
replica_list = [replica, active_replica, fake_replica(id=3)]
|
replica_list = [
|
||||||
|
replica, active_replica, fake_replica(id=3),
|
||||||
|
fake_replica(id='one_more_replica'),
|
||||||
|
]
|
||||||
updated_replica_list = [
|
updated_replica_list = [
|
||||||
{
|
{
|
||||||
'id': replica['id'],
|
'id': replica['id'],
|
||||||
@ -1162,7 +1165,12 @@ class ShareManagerTestCase(test.TestCase):
|
|||||||
},
|
},
|
||||||
{
|
{
|
||||||
'id': 'other_replica',
|
'id': 'other_replica',
|
||||||
'export_locations': ['TEST1', 'TEST2'],
|
'export_locations': ['TEST3', 'TEST4'],
|
||||||
|
},
|
||||||
|
{
|
||||||
|
'id': replica_list[3]['id'],
|
||||||
|
'export_locations': ['TEST5', 'TEST6'],
|
||||||
|
'replica_state': constants.REPLICA_STATE_IN_SYNC,
|
||||||
},
|
},
|
||||||
]
|
]
|
||||||
self.mock_object(db, 'share_replica_get',
|
self.mock_object(db, 'share_replica_get',
|
||||||
@ -1200,13 +1208,19 @@ class ShareManagerTestCase(test.TestCase):
|
|||||||
demoted_replica_update_call = mock.call(
|
demoted_replica_update_call = mock.call(
|
||||||
mock.ANY, active_replica['id'], demoted_replica_updates
|
mock.ANY, active_replica['id'], demoted_replica_updates
|
||||||
)
|
)
|
||||||
|
additional_replica_update_call = mock.call(
|
||||||
|
mock.ANY, replica_list[3]['id'], {
|
||||||
|
'replica_state': constants.REPLICA_STATE_IN_SYNC,
|
||||||
|
}
|
||||||
|
)
|
||||||
|
|
||||||
self.share_manager.promote_share_replica(self.context, replica)
|
self.share_manager.promote_share_replica(self.context, replica)
|
||||||
|
|
||||||
self.assertEqual(2, mock_export_locs_update.call_count)
|
self.assertEqual(3, mock_export_locs_update.call_count)
|
||||||
self.assertEqual(2, mock_replica_update.call_count)
|
|
||||||
mock_replica_update.assert_has_calls([
|
mock_replica_update.assert_has_calls([
|
||||||
reset_replication_change_call, demoted_replica_update_call,
|
demoted_replica_update_call,
|
||||||
|
additional_replica_update_call,
|
||||||
|
reset_replication_change_call,
|
||||||
])
|
])
|
||||||
self.assertTrue(mock_info_log.called)
|
self.assertTrue(mock_info_log.called)
|
||||||
self.assertFalse(mock_snap_instance_update.called)
|
self.assertFalse(mock_snap_instance_update.called)
|
||||||
|
@ -0,0 +1,9 @@
|
|||||||
|
---
|
||||||
|
fixes:
|
||||||
|
- Fixed share replica status update concurrency in share replica promotion
|
||||||
|
feature. Before it was possible to see two active replicas,
|
||||||
|
having 'dr' or 'readable' type of replication,
|
||||||
|
performing 'share replica promotion' action.
|
||||||
|
Now, replica that becomes active is always updated last, so, at some period
|
||||||
|
of time we will have zero 'active' replicas at once instead of
|
||||||
|
two of them.
|
Loading…
Reference in New Issue
Block a user