diff --git a/manila/share/manager.py b/manila/share/manager.py index 94d2b34a..a03b4c16 100644 --- a/manila/share/manager.py +++ b/manila/share/manager.py @@ -1996,19 +1996,26 @@ class ShareManager(manager.SchedulerDependentManager): {'status': constants.STATUS_ERROR}) 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( context, old_active_replica['id'], {'replica_state': constants.REPLICA_STATE_OUT_OF_SYNC, 'cast_rules_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: - for updated_replica in updated_replica_list: + while updated_replica_list: + # NOTE(vponomaryov): update 'active' replica last. + 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( 'export_locations') if(updated_export_locs is not None diff --git a/manila/tests/share/test_manager.py b/manila/tests/share/test_manager.py index e430f432..d082eb52 100644 --- a/manila/tests/share/test_manager.py +++ b/manila/tests/share/test_manager.py @@ -1148,7 +1148,10 @@ class ShareManagerTestCase(test.TestCase): active_replica = fake_replica( id='current_active_replica', 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 = [ { 'id': replica['id'], @@ -1162,7 +1165,12 @@ class ShareManagerTestCase(test.TestCase): }, { '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', @@ -1200,13 +1208,19 @@ class ShareManagerTestCase(test.TestCase): demoted_replica_update_call = mock.call( 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.assertEqual(2, mock_export_locs_update.call_count) - self.assertEqual(2, mock_replica_update.call_count) + self.assertEqual(3, mock_export_locs_update.call_count) 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.assertFalse(mock_snap_instance_update.called) diff --git a/releasenotes/notes/bug-1664201-fix-share-replica-status-update-concurrency-in-replica-promotion-feature-63b15d96106c65da.yaml b/releasenotes/notes/bug-1664201-fix-share-replica-status-update-concurrency-in-replica-promotion-feature-63b15d96106c65da.yaml new file mode 100644 index 00000000..131c2145 --- /dev/null +++ b/releasenotes/notes/bug-1664201-fix-share-replica-status-update-concurrency-in-replica-promotion-feature-63b15d96106c65da.yaml @@ -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.