diff --git a/datastore_server/source/java/ch/systemsx/cisd/etlserver/plugins/SimpleShufflingShareFinder.java b/datastore_server/source/java/ch/systemsx/cisd/etlserver/plugins/SimpleShufflingShareFinder.java index 54255efb141079202c22edcddaad15c6378c7833..041895c028ccdec1b3b4790c6e98b60a55c9665a 100644 --- a/datastore_server/source/java/ch/systemsx/cisd/etlserver/plugins/SimpleShufflingShareFinder.java +++ b/datastore_server/source/java/ch/systemsx/cisd/etlserver/plugins/SimpleShufflingShareFinder.java @@ -23,7 +23,7 @@ import org.apache.commons.io.FileUtils; import ch.rinn.restrictions.Private; import ch.systemsx.cisd.common.utilities.PropertyUtils; -import ch.systemsx.cisd.openbis.dss.generic.shared.IShareFinder; +import ch.systemsx.cisd.openbis.dss.generic.shared.AbstractShareFinder; import ch.systemsx.cisd.openbis.dss.generic.shared.utils.Share; import ch.systemsx.cisd.openbis.generic.shared.dto.SimpleDataSetInformationDTO; @@ -32,7 +32,7 @@ import ch.systemsx.cisd.openbis.generic.shared.dto.SimpleDataSetInformationDTO; * * @author Franz-Josef Elmer */ -public class SimpleShufflingShareFinder implements IShareFinder +public class SimpleShufflingShareFinder extends AbstractShareFinder { @Private public static final String MINIMUM_FREE_SPACE_KEY = "minimum-free-space-in-MB"; @@ -46,12 +46,18 @@ public class SimpleShufflingShareFinder implements IShareFinder } - public Share tryToFindShare(SimpleDataSetInformationDTO dataSet, List<Share> shares) + @Override + protected Share tryToFindShare(SimpleDataSetInformationDTO dataSet, List<Share> shares, + ISpeedChecker speedChecker) { Share shareWithMostFree = null; long maxFreeSpace = 0; for (Share share : shares) { + if (speedChecker.check(dataSet, share) == false) + { + continue; + } long freeSpace = share.calculateFreeSpace(); if (freeSpace > maxFreeSpace) { diff --git a/datastore_server/source/java/ch/systemsx/cisd/etlserver/registrator/api/v1/IDataSet.java b/datastore_server/source/java/ch/systemsx/cisd/etlserver/registrator/api/v1/IDataSet.java index b3e50fa9898ab1a97485c152618faf142201c09f..cb6184b35860111995e1307d297b589ab10d1353 100644 --- a/datastore_server/source/java/ch/systemsx/cisd/etlserver/registrator/api/v1/IDataSet.java +++ b/datastore_server/source/java/ch/systemsx/cisd/etlserver/registrator/api/v1/IDataSet.java @@ -114,10 +114,8 @@ public interface IDataSet * absolute value less than or equal {@link Constants#MAX_SPEED}. * <p> * A positive value means that the data set should be stored in a storage with speed >= - * <code>speedHint</code>. - * <p> - * A negative value means that the data set should be stored in a storage with speed <= - * <code>abs(speedHint)</code>. + * <code>speedHint</code>. A negative value means that the data set should be stored in a + * storage with speed <= <code>abs(speedHint)</code>. The speed hint might be ignored. */ public void setSpeedHint(int speedHint); diff --git a/datastore_server/source/java/ch/systemsx/cisd/openbis/dss/generic/shared/AbstractShareFinder.java b/datastore_server/source/java/ch/systemsx/cisd/openbis/dss/generic/shared/AbstractShareFinder.java new file mode 100644 index 0000000000000000000000000000000000000000..4e2609bf7331e1773473678f80aedc786a31c5e1 --- /dev/null +++ b/datastore_server/source/java/ch/systemsx/cisd/openbis/dss/generic/shared/AbstractShareFinder.java @@ -0,0 +1,108 @@ +/* + * Copyright 2011 ETH Zuerich, CISD + * + * Licensed 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 ch.systemsx.cisd.openbis.dss.generic.shared; + +import java.util.List; + +import ch.systemsx.cisd.openbis.dss.generic.shared.utils.Share; +import ch.systemsx.cisd.openbis.generic.shared.dto.SimpleDataSetInformationDTO; + +/** + * Abstract implementation which fulfills the contract of {@link IShareFinder} concerning speed + * hint. + * + * @author Franz-Josef Elmer + */ +public abstract class AbstractShareFinder implements IShareFinder +{ + /** + * Returns <code>true</code> if speed of specified share and speed hint of specified data set + * are allowed. + * + * @author Franz-Josef Elmer + */ + protected static interface ISpeedChecker + { + boolean check(SimpleDataSetInformationDTO dataSet, Share share); + } + + private enum SpeedChecker implements ISpeedChecker + { + MATCHING_CHECKER() + { + public boolean check(SimpleDataSetInformationDTO dataSet, Share share) + { + return Math.abs(dataSet.getSpeedHint()) == share.getSpeed(); + } + }, + RESPECTUNG_SPEED_HINT_CHECKER() + { + public boolean check(SimpleDataSetInformationDTO dataSet, Share share) + { + int speedHint = dataSet.getSpeedHint(); + int speed = share.getSpeed(); + return speedHint < 0 ? speed < Math.abs(speedHint) : speed > speedHint; + } + }, + IGNORING_SPEED_HINT_CHECKER() + { + public boolean check(SimpleDataSetInformationDTO dataSet, Share share) + { + return true; + } + } + } + + /** + * Tries to find a share fulfilling speed hint contract. + */ + public Share tryToFindShare(SimpleDataSetInformationDTO dataSet, List<Share> shares) + { + SpeedChecker[] values = SpeedChecker.values(); + for (SpeedChecker speedChecker : values) + { + Share share = tryToFindShare(dataSet, shares, speedChecker); + if (share != null) + { + assertSpeedChecker(dataSet, share, speedChecker); + return share; + } + } + return null; + } + + private void assertSpeedChecker(SimpleDataSetInformationDTO dataSet, Share share, + SpeedChecker speedChecker) + { + if (speedChecker.check(dataSet, share) == false) + { + throw new AssertionError("Found share " + share.getShareId() + " has speed " + + share.getSpeed() + " but data set " + dataSet.getDataSetCode() + + " has speed hint " + dataSet.getSpeedHint() + + ". This violates speed checker " + speedChecker + "."); + } + } + + /** + * Tries to find a share from the specified shares to whom the specified data set can be moved. + * The returned share has to fulfill specified speed checker. That is, if return value + * <code>share != null</code> then <code>speedChecker(dataSet, share) == true</code>. + */ + protected abstract Share tryToFindShare(SimpleDataSetInformationDTO dataSet, + List<Share> shares, ISpeedChecker speedChecker); + +} diff --git a/datastore_server/source/java/ch/systemsx/cisd/openbis/dss/generic/shared/IShareFinder.java b/datastore_server/source/java/ch/systemsx/cisd/openbis/dss/generic/shared/IShareFinder.java index f5fd5bf72a17549b7da6899fa97495ce799182a2..bd79fc29fdac3cb500eec2a608dd829c03064eb0 100644 --- a/datastore_server/source/java/ch/systemsx/cisd/openbis/dss/generic/shared/IShareFinder.java +++ b/datastore_server/source/java/ch/systemsx/cisd/openbis/dss/generic/shared/IShareFinder.java @@ -34,7 +34,8 @@ public interface IShareFinder * Tries to find a share from the specified shares to whom the specified data set can be moved. * Implementations should choose a share with speed matching the absolute value of the speed * hint of specified data set. If such a share couldn't be found a share with higher/lower speed - * should be chosen if speed hint is positive/negative. + * should be chosen if speed hint is positive/negative. In worst case speed hint is completely + * ignored. * * @param dataSet with known size, old share ID and speed hint. * @param shares All shares. Share instances know their speed and whether they are incoming or diff --git a/datastore_server/source/java/ch/systemsx/cisd/openbis/dss/generic/shared/SimpleShareFinder.java b/datastore_server/source/java/ch/systemsx/cisd/openbis/dss/generic/shared/SimpleShareFinder.java index 12680609b8b8fd56941f6d9e66eed738b355fa65..180f056c13b9aa90d9c44de329bb2b30f8c2a07a 100644 --- a/datastore_server/source/java/ch/systemsx/cisd/openbis/dss/generic/shared/SimpleShareFinder.java +++ b/datastore_server/source/java/ch/systemsx/cisd/openbis/dss/generic/shared/SimpleShareFinder.java @@ -30,14 +30,16 @@ import ch.systemsx.cisd.openbis.generic.shared.dto.SimpleDataSetInformationDTO; * * @author Franz-Josef Elmer */ -public class SimpleShareFinder implements IShareFinder +public class SimpleShareFinder extends AbstractShareFinder { public SimpleShareFinder(Properties properties) { } - public Share tryToFindShare(SimpleDataSetInformationDTO dataSet, List<Share> shares) + @Override + protected Share tryToFindShare(SimpleDataSetInformationDTO dataSet, List<Share> shares, + ISpeedChecker speedChecker) { Long dataSetSize = dataSet.getDataSetSize(); String dataSetShareId = dataSet.getDataSetShareId(); @@ -49,6 +51,10 @@ public class SimpleShareFinder implements IShareFinder long extensionsMaxFreeSpace = dataSetSize; for (Share share : shares) { + if (speedChecker.check(dataSet, share) == false) + { + continue; + } long freeSpace = share.calculateFreeSpace(); String shareId = share.getShareId(); if (dataSetShareId.equals(shareId)) diff --git a/datastore_server/sourceTest/java/ch/systemsx/cisd/openbis/dss/generic/shared/AbstractShareFinderTest.java b/datastore_server/sourceTest/java/ch/systemsx/cisd/openbis/dss/generic/shared/AbstractShareFinderTest.java new file mode 100644 index 0000000000000000000000000000000000000000..2b0df210a62f0764a870df09e3367e48d83b0a42 --- /dev/null +++ b/datastore_server/sourceTest/java/ch/systemsx/cisd/openbis/dss/generic/shared/AbstractShareFinderTest.java @@ -0,0 +1,209 @@ +/* + * Copyright 2011 ETH Zuerich, CISD + * + * Licensed 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 ch.systemsx.cisd.openbis.dss.generic.shared; + +import java.io.File; +import java.util.Collections; +import java.util.List; + +import org.testng.AssertJUnit; +import org.testng.annotations.Test; + +import ch.systemsx.cisd.openbis.dss.generic.shared.utils.Share; +import ch.systemsx.cisd.openbis.generic.shared.dto.SimpleDataSetInformationDTO; + +/** + * + * + * @author Franz-Josef Elmer + */ +public class AbstractShareFinderTest extends AssertJUnit +{ + private static final String DATA_SET_CODE = "ds-1"; + private static final List<Share> SHARES = Collections.emptyList(); + + private static final class MockShareFinder extends AbstractShareFinder + { + private final Share[] returnValues; + + private int index; + private SimpleDataSetInformationDTO recordedDataSet; + private List<Share> recordedShares; + + MockShareFinder(Share... shares) + { + this.returnValues = shares; + } + + @Override + protected Share tryToFindShare(SimpleDataSetInformationDTO dataSet, List<Share> shares, + ISpeedChecker speedChecker) + { + recordedDataSet = dataSet; + recordedShares = shares; + return returnValues[index++]; + } + + void verify(SimpleDataSetInformationDTO dataSet, List<Share> shares) + { + assertSame(dataSet, recordedDataSet); + assertSame(shares, recordedShares); + assertEquals(index, returnValues.length); + } + } + + @Test + public void testFindMatchingShareForPositiveHint() + { + Share share = share(20); + MockShareFinder finder = new MockShareFinder(share); + + SimpleDataSetInformationDTO dataSet = dataSet(20); + Share foundShare = finder.tryToFindShare(dataSet, SHARES); + + assertSame(share, foundShare); + finder.verify(dataSet, SHARES); + } + + @Test + public void testFindMatchingShareForNegativeHint() + { + Share share = share(20); + MockShareFinder finder = new MockShareFinder(share); + + SimpleDataSetInformationDTO dataSet = dataSet(-20); + Share foundShare = finder.tryToFindShare(dataSet, SHARES); + + assertSame(share, foundShare); + finder.verify(dataSet, SHARES); + } + + @Test + public void testFindInvalidMatchingShare() + { + Share share = share(20); + MockShareFinder finder = new MockShareFinder(share); + + SimpleDataSetInformationDTO dataSet = dataSet(21); + try + { + finder.tryToFindShare(dataSet, SHARES); + fail("AssertionError expected"); + } catch (AssertionError ex) + { + assertEquals("Found share 2 has speed 20 but data set ds-1 has speed hint 21. " + + "This violates speed checker MATCHING_CHECKER.", ex.getMessage()); + } + finder.verify(dataSet, SHARES); + } + + @Test + public void testFindRespectingShareForPositiveHint() + { + Share share = share(21); + MockShareFinder finder = new MockShareFinder(null, share); + + SimpleDataSetInformationDTO dataSet = dataSet(20); + Share foundShare = finder.tryToFindShare(dataSet, SHARES); + + assertSame(share, foundShare); + finder.verify(dataSet, SHARES); + } + + @Test + public void testFindRespectingShareForNegativeHint() + { + Share share = share(19); + MockShareFinder finder = new MockShareFinder(null, share); + + SimpleDataSetInformationDTO dataSet = dataSet(-20); + Share foundShare = finder.tryToFindShare(dataSet, SHARES); + + assertSame(share, foundShare); + finder.verify(dataSet, SHARES); + } + + @Test + public void testFindInvalidRespectingShareForPositiveHint() + { + Share share = share(20); + MockShareFinder finder = new MockShareFinder(null, share); + + final SimpleDataSetInformationDTO dataSet = dataSet(20); + try + { + finder.tryToFindShare(dataSet, SHARES); + fail("AssertionError expected"); + } catch (AssertionError ex) + { + assertEquals("Found share 2 has speed 20 but data set ds-1 has speed hint 20. " + + "This violates speed checker RESPECTUNG_SPEED_HINT_CHECKER.", ex.getMessage()); + } + finder.verify(dataSet, SHARES); + } + + @Test + public void testFindAnyShareForPositiveHint() + { + Share share = share(10); + MockShareFinder finder = new MockShareFinder(null, null, share); + + SimpleDataSetInformationDTO dataSet = dataSet(20); + Share foundShare = finder.tryToFindShare(dataSet, SHARES); + + assertSame(share, foundShare); + finder.verify(dataSet, SHARES); + } + + @Test + public void testFindAnyShareForNegativeHint() + { + Share share = share(10); + MockShareFinder finder = new MockShareFinder(null, null, share); + + SimpleDataSetInformationDTO dataSet = dataSet(-20); + Share foundShare = finder.tryToFindShare(dataSet, SHARES); + + assertSame(share, foundShare); + finder.verify(dataSet, SHARES); + } + + @Test + public void testFindNoShare() + { + MockShareFinder finder = new MockShareFinder(null, null, null); + + SimpleDataSetInformationDTO dataSet = dataSet(-20); + Share foundShare = finder.tryToFindShare(dataSet, SHARES); + + assertSame(null, foundShare); + finder.verify(dataSet, SHARES); + } + + private SimpleDataSetInformationDTO dataSet(int speedHint) + { + SimpleDataSetInformationDTO dataSet = new SimpleDataSetInformationDTO(); + dataSet.setDataSetCode(DATA_SET_CODE); + dataSet.setSpeedHint(speedHint); + return dataSet; + } + + private Share share(int speed) + { + return new Share(new File(Integer.toString(speed / 10)), speed, null); + } +}