Skip to content

Fix the problem at #1740 when it loads all snapshots in the primary storage #2808

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
rafaelweingartner merged 2 commits into from
Aug 17, 2018
Merged

Fix the problem at #1740 when it loads all snapshots in the primary storage #2808

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
41f9058
Fix the problem at #1740 when it loads all snapshots in the primary s…
rafaelweingartner Aug 15, 2018
842baad
Add unit test
rafaelweingartner Aug 16, 2018
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 6 additions & 1 deletion ...schema/src/main/java/org/apache/cloudstack/storage/datastore/db/SnapshotDataStoreDao.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@
import com.cloud.utils.fsm.StateDao;

public interface SnapshotDataStoreDao extends GenericDao<SnapshotDataStoreVO, Long>,
StateDao<ObjectInDataStoreStateMachine.State, ObjectInDataStoreStateMachine.Event, DataObjectInStore> {
StateDao<ObjectInDataStoreStateMachine.State, ObjectInDataStoreStateMachine.Event, DataObjectInStore> {

List<SnapshotDataStoreVO> listByStoreId(long id, DataStoreRole role);

Expand Down Expand Up @@ -63,5 +63,10 @@ public interface SnapshotDataStoreDao extends GenericDao<SnapshotDataStoreVO, Lo

SnapshotDataStoreVO findByVolume(long volumeId, DataStoreRole role);

/**
* List all snapshots in 'snapshot_store_ref' by volume and data store role. Therefore, it is possible to list all snapshots that are in the primary storage or in the secondary storage.
*/
List<SnapshotDataStoreVO> listAllByVolumeAndDataStore(long volumeId, DataStoreRole role);

List<SnapshotDataStoreVO> listByState(ObjectInDataStoreStateMachine.State... states);
}
23 changes: 11 additions & 12 deletions ...napshot/src/main/java/org/apache/cloudstack/storage/snapshot/SnapshotDataFactoryImpl.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,24 +28,23 @@
import org.apache.cloudstack.engine.subsystem.api.storage.DataStoreManager;
import org.apache.cloudstack.engine.subsystem.api.storage.SnapshotDataFactory;
import org.apache.cloudstack.engine.subsystem.api.storage.SnapshotInfo;
import org.apache.cloudstack.engine.subsystem.api.storage.VolumeDataFactory;
import org.apache.cloudstack.storage.datastore.db.SnapshotDataStoreDao;
import org.apache.cloudstack.storage.datastore.db.SnapshotDataStoreVO;
import org.apache.commons.collections.CollectionUtils;

import com.cloud.storage.DataStoreRole;
import com.cloud.storage.SnapshotVO;
import com.cloud.storage.dao.SnapshotDao;
import com.cloud.utils.exception.CloudRuntimeException;

public class SnapshotDataFactoryImpl implements SnapshotDataFactory {

@Inject
SnapshotDao snapshotDao;
@Inject
SnapshotDataStoreDao snapshotStoreDao;
private SnapshotDao snapshotDao;
@Inject
DataStoreManager storeMgr;
private SnapshotDataStoreDao snapshotStoreDao;
@Inject
VolumeDataFactory volumeFactory;
private DataStoreManager storeMgr;

@Override
public SnapshotInfo getSnapshot(long snapshotId, DataStore store) {
Expand All @@ -66,16 +65,16 @@ public SnapshotInfo getSnapshot(DataObject obj, DataStore store) {

@Override
public List<SnapshotInfo> getSnapshots(long volumeId, DataStoreRole role) {

SnapshotDataStoreVO snapshotStore = snapshotStoreDao.findByVolume(volumeId, role);
if (snapshotStore == null) {
List<SnapshotDataStoreVO> allSnapshotsFromVolumeAndDataStore = snapshotStoreDao.listAllByVolumeAndDataStore(volumeId, role);
if (CollectionUtils.isEmpty(allSnapshotsFromVolumeAndDataStore)) {
return new ArrayList<>();
}
DataStore store = storeMgr.getDataStore(snapshotStore.getDataStoreId(), role);
List<SnapshotVO> volSnapShots = snapshotDao.listByVolumeId(volumeId);
List<SnapshotInfo> infos = new ArrayList<>();
for(SnapshotVO snapshot: volSnapShots) {
for (SnapshotDataStoreVO snapshotDataStoreVO : allSnapshotsFromVolumeAndDataStore) {
DataStore store = storeMgr.getDataStore(snapshotDataStoreVO.getDataStoreId(), role);
SnapshotVO snapshot = snapshotDao.findById(snapshotDataStoreVO.getSnapshotId());
SnapshotObject info = SnapshotObject.getSnapshotObject(snapshot, store);

infos.add(info);
}
return infos;
Expand Down
110 changes: 110 additions & 0 deletions ...hot/src/test/java/org/apache/cloudstack/storage/snapshot/SnapshotDataFactoryImplTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,110 @@
/*
* Licensed to the Apache Software Foundation (ASF) under one
* or more contributor license agreements. See the NOTICE file
* distributed with this work for additional information
* regarding copyright ownership. The ASF licenses this file
* to you under the Apache License, Version 2.0 (the
* "License"); you may not use this file except in compliance
* with the License. You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing,
* software distributed under the License is distributed on an
* "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
* KIND, either express or implied. See the License for the
* specific language governing permissions and limitations
* under the License.
*/
package org.apache.cloudstack.storage.snapshot;

import java.util.ArrayList;
import java.util.List;

import org.apache.cloudstack.engine.subsystem.api.storage.DataStore;
import org.apache.cloudstack.engine.subsystem.api.storage.DataStoreManager;
import org.apache.cloudstack.engine.subsystem.api.storage.SnapshotInfo;
import org.apache.cloudstack.storage.datastore.db.SnapshotDataStoreDao;
import org.apache.cloudstack.storage.datastore.db.SnapshotDataStoreVO;
import org.junit.Assert;
import org.junit.Test;
import org.junit.runner.RunWith;
import org.mockito.InjectMocks;
import org.mockito.Mock;
import org.mockito.Mockito;
import org.mockito.Spy;
import org.powermock.api.mockito.PowerMockito;
import org.powermock.core.classloader.annotations.PrepareForTest;
import org.powermock.modules.junit4.PowerMockRunner;

import com.cloud.storage.DataStoreRole;
import com.cloud.storage.SnapshotVO;
import com.cloud.storage.dao.SnapshotDao;
import com.cloud.utils.component.ComponentContext;

@RunWith(PowerMockRunner.class)
public class SnapshotDataFactoryImplTest {

@Spy
@InjectMocks
private SnapshotDataFactoryImpl snapshotDataFactoryImpl = new SnapshotDataFactoryImpl();

@Mock
private SnapshotDataStoreDao snapshotStoreDaoMock;
@Mock
private DataStoreManager dataStoreManagerMock;
@Mock
private SnapshotDao snapshotDaoMock;

private long volumeMockId = 1;

@Test
public void getSnapshotsByVolumeAndDataStoreTestNoSnapshotDataStoreVOFound() {
Mockito.doReturn(new ArrayList<>()).when(snapshotStoreDaoMock).listAllByVolumeAndDataStore(volumeMockId, DataStoreRole.Primary);

List<SnapshotInfo> snapshots = snapshotDataFactoryImpl.getSnapshots(volumeMockId, DataStoreRole.Primary);

Assert.assertTrue(snapshots.isEmpty());
}

@Test
@PrepareForTest({ComponentContext.class})
public void getSnapshotsByVolumeAndDataStoreTest() {
PowerMockito.mockStatic(ComponentContext.class);
PowerMockito.when(ComponentContext.inject(SnapshotObject.class)).thenReturn(new SnapshotObject());

SnapshotDataStoreVO snapshotDataStoreVoMock = Mockito.mock(SnapshotDataStoreVO.class);
Mockito.doReturn(volumeMockId).when(snapshotDataStoreVoMock).getVolumeId();

long snapshotId = 1223;
long dataStoreId = 34567;
Mockito.doReturn(snapshotId).when(snapshotDataStoreVoMock).getSnapshotId();
Mockito.doReturn(dataStoreId).when(snapshotDataStoreVoMock).getDataStoreId();

SnapshotVO snapshotVoMock = Mockito.mock(SnapshotVO.class);
Mockito.doReturn(snapshotId).when(snapshotVoMock).getId();

DataStoreRole dataStoreRole = DataStoreRole.Primary;
DataStore dataStoreMock = Mockito.mock(DataStore.class);
Mockito.doReturn(dataStoreId).when(dataStoreMock).getId();
Mockito.doReturn(dataStoreRole).when(dataStoreMock).getRole();

List<SnapshotDataStoreVO> snapshotDataStoreVOs = new ArrayList<>();
snapshotDataStoreVOs.add(snapshotDataStoreVoMock);

Mockito.doReturn(snapshotDataStoreVOs).when(snapshotStoreDaoMock).listAllByVolumeAndDataStore(volumeMockId, dataStoreRole);
Mockito.doReturn(dataStoreMock).when(dataStoreManagerMock).getDataStore(dataStoreId, dataStoreRole);
Mockito.doReturn(snapshotVoMock).when(snapshotDaoMock).findById(snapshotId);

List<SnapshotInfo> snapshots = snapshotDataFactoryImpl.getSnapshots(volumeMockId, dataStoreRole);

Assert.assertEquals(1, snapshots.size());

SnapshotInfo snapshotInfo = snapshots.get(0);
Assert.assertEquals(dataStoreMock, snapshotInfo.getDataStore());
Assert.assertEquals(snapshotVoMock, ((SnapshotObject)snapshotInfo).getSnapshotVO());

PowerMockito.verifyStatic();
ComponentContext.inject(SnapshotObject.class);
}
}
100 changes: 55 additions & 45 deletions ...torage/src/main/java/org/apache/cloudstack/storage/image/db/SnapshotDataStoreDaoImpl.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,14 +16,17 @@
// under the License.
package org.apache.cloudstack.storage.image.db;

import com.cloud.storage.DataStoreRole;
import com.cloud.utils.db.DB;
import com.cloud.utils.db.GenericDaoBase;
import com.cloud.utils.db.SearchBuilder;
import com.cloud.utils.db.SearchCriteria;
import com.cloud.utils.db.SearchCriteria.Op;
import com.cloud.utils.db.TransactionLegacy;
import com.cloud.utils.db.UpdateBuilder;
import java.sql.PreparedStatement;
import java.sql.ResultSet;
import java.sql.SQLException;
import java.util.ArrayList;
import java.util.Date;
import java.util.List;
import java.util.Map;

import javax.inject.Inject;
import javax.naming.ConfigurationException;

import org.apache.cloudstack.engine.subsystem.api.storage.DataObjectInStore;
import org.apache.cloudstack.engine.subsystem.api.storage.ObjectInDataStoreStateMachine;
import org.apache.cloudstack.engine.subsystem.api.storage.ObjectInDataStoreStateMachine.Event;
Expand All @@ -33,19 +36,18 @@
import org.apache.log4j.Logger;
import org.springframework.stereotype.Component;

import com.cloud.hypervisor.Hypervisor;
import com.cloud.storage.DataStoreRole;
import com.cloud.storage.SnapshotVO;
import com.cloud.storage.dao.SnapshotDao;
import javax.inject.Inject;
import com.cloud.hypervisor.Hypervisor;
import java.util.ArrayList;
import com.cloud.utils.db.DB;
import com.cloud.utils.db.Filter;
import javax.naming.ConfigurationException;
import java.sql.PreparedStatement;
import java.sql.ResultSet;
import java.sql.SQLException;
import java.util.Date;
import java.util.List;
import java.util.Map;
import com.cloud.utils.db.GenericDaoBase;
import com.cloud.utils.db.SearchBuilder;
import com.cloud.utils.db.SearchCriteria;
import com.cloud.utils.db.SearchCriteria.Op;
import com.cloud.utils.db.TransactionLegacy;
import com.cloud.utils.db.UpdateBuilder;

@Component
public class SnapshotDataStoreDaoImpl extends GenericDaoBase<SnapshotDataStoreVO, Long> implements SnapshotDataStoreDao {
Expand Down Expand Up @@ -176,33 +178,33 @@ public boolean updateState(State currentState, Event event, State nextState, Dat
if (dbVol != null) {
StringBuilder str = new StringBuilder("Unable to update ").append(dataObj.toString());
str.append(": DB Data={id=")
.append(dbVol.getId())
.append("; state=")
.append(dbVol.getState())
.append("; updatecount=")
.append(dbVol.getUpdatedCount())
.append(";updatedTime=")
.append(dbVol.getUpdated());
.append(dbVol.getId())
.append("; state=")
.append(dbVol.getState())
.append("; updatecount=")
.append(dbVol.getUpdatedCount())
.append(";updatedTime=")
.append(dbVol.getUpdated());
str.append(": New Data={id=")
.append(dataObj.getId())
.append("; state=")
.append(nextState)
.append("; event=")
.append(event)
.append("; updatecount=")
.append(dataObj.getUpdatedCount())
.append("; updatedTime=")
.append(dataObj.getUpdated());
.append(dataObj.getId())
.append("; state=")
.append(nextState)
.append("; event=")
.append(event)
.append("; updatecount=")
.append(dataObj.getUpdatedCount())
.append("; updatedTime=")
.append(dataObj.getUpdated());
str.append(": stale Data={id=")
.append(dataObj.getId())
.append("; state=")
.append(currentState)
.append("; event=")
.append(event)
.append("; updatecount=")
.append(oldUpdated)
.append("; updatedTime=")
.append(oldUpdatedTime);
.append(dataObj.getId())
.append("; state=")
.append(currentState)
.append("; event=")
.append(event)
.append("; updatecount=")
.append(oldUpdated)
.append("; updatedTime=")
.append(oldUpdatedTime);
} else {
s_logger.debug("Unable to update objectIndatastore: id=" + dataObj.getId() + ", as there is no such object exists in the database anymore");
}
Expand Down Expand Up @@ -254,7 +256,7 @@ public SnapshotDataStoreVO findLatestSnapshotForVolume(Long volumeId, DataStoreR
TransactionLegacy txn = TransactionLegacy.currentTxn();
try (
PreparedStatement pstmt = txn.prepareStatement(findLatestSnapshot);
){
){
pstmt.setString(1, role.toString());
pstmt.setLong(2, volumeId);
try (ResultSet rs = pstmt.executeQuery();) {
Expand All @@ -275,7 +277,7 @@ public SnapshotDataStoreVO findOldestSnapshotForVolume(Long volumeId, DataStoreR
TransactionLegacy txn = TransactionLegacy.currentTxn();
try (
PreparedStatement pstmt = txn.prepareStatement(findOldestSnapshot);
){
){
pstmt.setString(1, role.toString());
pstmt.setLong(2, volumeId);
try (ResultSet rs = pstmt.executeQuery();) {
Expand Down Expand Up @@ -318,6 +320,14 @@ public SnapshotDataStoreVO findBySnapshot(long snapshotId, DataStoreRole role) {
return findOneBy(sc);
}

@Override
public List<SnapshotDataStoreVO> listAllByVolumeAndDataStore(long volumeId, DataStoreRole role) {
SearchCriteria<SnapshotDataStoreVO> sc = volumeSearch.create();
sc.setParameters("volume_id", volumeId);
sc.setParameters("store_role", role);
return listBy(sc);
}

@Override
public SnapshotDataStoreVO findByVolume(long volumeId, DataStoreRole role) {
SearchCriteria<SnapshotDataStoreVO> sc = volumeSearch.create();
Expand Down