8:10 a.m.
From Tal Nisan <tnisan(a)redhat.com>:
Tal Nisan has submitted this change and it was merged.
Change subject: backend: new exclusive lock for SyncLunsCommand
......................................................................
backend: new exclusive lock for SyncLunsCommand
This patch replaces the group of the exclusive lock that is used in
SyncLunsInfoForBlockStorageDomainCommand (hereafter called
"SyncLunsCommand") from SYNC_LUNS to STORAGE in order to avoid running
it in parallel with ExtendSANStorageDomainCommand and
ReduceSANStorageDomainDevicesCommand.
To understand why it would corrupt the db, think about the following
scenario:
1. There exists a block storage domain, sd1, and it is composed from one
lun, lun1.
1. Thread A enters SyncLunsCommand::executeCommand, performs the call to
getLunsFromVgInfo() and stores the information about lun1 in
lunsFromVgInfo.
2. Thread B starts and completes the execution of
ExtendSANStorageDomainCommand, so now sd1 is composed from two luns,
lun1 and lun2.
3. Thread A executes the next line and stores the luns of sd1, lun1 and
lun2, in lunsFromDb.
4. Thread A continues and at the end removes lun2 from the luns table,
since it assumes that it's an unused and irrelevant lun.
A similar case would happen with ReduceSANStorageDomainDevicesCommand
(the only difference is that we will end up with another lun that should
not have remained in the db).
Since 13 other commands use a lock with the group STORAGE, and since we
should make sure that some important commands won't get stuck when
running in parallel with SyncLunsCommand, here's a list of all them:
**Irrelevant commands**
Following are commands that can't run in parallel with
SyncLunsCommand because they will be blocked in the can do action
anyway. Remember that executeCommand requires an active block domain.
1. DetachStorageDomainFromPoolCommand - sd should be on maintenance.
2. ForceRemoveStorageDomainCommand - sd cannot be active.
3. RemoveStorageDomainCommand - sd should be detached.
4. ImportHostedEngineStorageDomainCommand - sd still doesn't exist.
5. ActivateStorageDomainCommand - calls to SyncLunsCommand right after
freeing the STORAGE lock.
6. AddExistingBlockStorageDomainCommand - sd still doesn't exist.
7. AttachStorageDomainToPoolCommand - sd should be detached.
**Commands that we don't want to run in parallel with SyncLunsCommand**
1. ExtendSANStorageDomainCommand
2. ReduceSANStorageDomainDevicesCommand
**Other commands that will collide with SyncLunsCommand**
Following are commands that will collide with SyncLunsCommand, and thus
the first command that will get the lock will succeed to run:
1. DeactivateStorageDomainCommand
2. ProcessOvfUpdateForStorageDomainCommand
3. ScanStorageForUnregisteredDisksCommand
4. UpdateStorageServerConnectionCommand
Since SyncLunsCommand is not system initiated but user initiated, IMHO
the risk that these commands will clash is presumably low.
Change-Id: Ie0d56c593c5847e164f90d885a6711cd77c9c486
Related-To: https://bugzilla.redhat.com/1455548
Signed-off-by: Idan Shaby <ishaby(a)redhat.com>
---
M
backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/storage/domain/SyncLunsInfoForBlockStorageDomainCommand.java
M
backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/locks/LockingGroup.java
2 files changed, 1 insertion(+), 2 deletions(-)
Approvals:
Jenkins CI: Passed CI tests
Allon Mureinik: Looks good to me, approved
Idan Shaby: Verified
--
To view, visit https://gerrit.ovirt.org/78056
To unsubscribe, visit https://gerrit.ovirt.org/settings
Gerrit-MessageType: merged
Gerrit-Change-Id: Ie0d56c593c5847e164f90d885a6711cd77c9c486
Gerrit-PatchSet: 2
Gerrit-Project: ovirt-engine
Gerrit-Branch: ovirt-engine-4.1
Gerrit-Owner: Idan Shaby <ishaby(a)redhat.com>
Gerrit-Reviewer: Allon Mureinik <amureini(a)redhat.com>
Gerrit-Reviewer: Daniel Erez <derez(a)redhat.com>
Gerrit-Reviewer: Idan Shaby <ishaby(a)redhat.com>
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Tal Nisan <tnisan(a)redhat.com>
Gerrit-Reviewer: gerrit-hooks <automation(a)ovirt.org>