This is an automated email from the ASF dual-hosted git repository. liyang pushed a commit to branch sync in repository https://gitbox.apache.org/repos/asf/kylin.git
commit 3222de27c508ed3e7576330dc8c40206fe6cd84d Author: Li Yang <liy...@apache.org> AuthorDate: Sun Mar 25 18:12:53 2018 +0800 KYLIN-3311 refactor Resource.checkAndPutResourceImpl() to throw WriteConflictException --- .../apache/kylin/common/persistence/FileResourceStore.java | 6 +++--- .../apache/kylin/common/persistence/HDFSResourceStore.java | 4 ++-- .../org/apache/kylin/common/persistence/ResourceStore.java | 12 ++++++++---- .../{StorageException.java => WriteConflictException.java} | 9 +++------ .../apache/kylin/common/persistence/ResourceStoreTest.java | 2 +- .../src/main/java/org/apache/kylin/cube/CubeManager.java | 3 ++- .../java/org/apache/kylin/metadata/TempStatementManager.java | 3 ++- .../main/java/org/apache/kylin/rest/service/AclService.java | 3 ++- .../java/org/apache/kylin/storage/hbase/HBaseConnection.java | 3 +-- .../org/apache/kylin/storage/hbase/HBaseResourceStore.java | 3 ++- 10 files changed, 26 insertions(+), 22 deletions(-) diff --git a/core-common/src/main/java/org/apache/kylin/common/persistence/FileResourceStore.java b/core-common/src/main/java/org/apache/kylin/common/persistence/FileResourceStore.java index 38ccbdd..7c6c506 100644 --- a/core-common/src/main/java/org/apache/kylin/common/persistence/FileResourceStore.java +++ b/core-common/src/main/java/org/apache/kylin/common/persistence/FileResourceStore.java @@ -31,13 +31,13 @@ import java.util.List; import java.util.NavigableSet; import java.util.TreeSet; -import com.google.common.base.Preconditions; import org.apache.commons.io.FileUtils; import org.apache.commons.io.IOUtils; import org.apache.kylin.common.KylinConfig; import org.slf4j.Logger; import org.slf4j.LoggerFactory; +import com.google.common.base.Preconditions; import com.google.common.collect.Lists; public class FileResourceStore extends ResourceStore { @@ -176,12 +176,12 @@ public class FileResourceStore extends ResourceStore { @Override protected long checkAndPutResourceImpl(String resPath, byte[] content, long oldTS, long newTS) - throws IOException, IllegalStateException { + throws IOException, WriteConflictException { synchronized (FileResourceStore.class) { File f = file(resPath); if ((f.exists() && f.lastModified() != oldTS) || (f.exists() == false && oldTS != 0)) - throw new IllegalStateException("Overwriting conflict " + resPath + ", expect old TS " + oldTS + throw new WriteConflictException("Overwriting conflict " + resPath + ", expect old TS " + oldTS + ", but found " + f.lastModified()); putResourceImpl(resPath, new ByteArrayInputStream(content), newTS); diff --git a/core-common/src/main/java/org/apache/kylin/common/persistence/HDFSResourceStore.java b/core-common/src/main/java/org/apache/kylin/common/persistence/HDFSResourceStore.java index 8ad2540..1739ce0 100644 --- a/core-common/src/main/java/org/apache/kylin/common/persistence/HDFSResourceStore.java +++ b/core-common/src/main/java/org/apache/kylin/common/persistence/HDFSResourceStore.java @@ -207,7 +207,7 @@ public class HDFSResourceStore extends ResourceStore { } @Override - protected long checkAndPutResourceImpl(String resPath, byte[] content, long oldTS, long newTS) throws IOException, IllegalStateException { + protected long checkAndPutResourceImpl(String resPath, byte[] content, long oldTS, long newTS) throws IOException, WriteConflictException { Path p = getRealHDFSPath(resPath); if (!fs.exists(p)) { if (oldTS != 0) { @@ -217,7 +217,7 @@ public class HDFSResourceStore extends ResourceStore { } else { long realLastModify = getResourceTimestamp(resPath); if (realLastModify != oldTS) { - throw new IllegalStateException("Overwriting conflict " + resPath + ", expect old TS " + oldTS + ", but found " + realLastModify); + throw new WriteConflictException("Overwriting conflict " + resPath + ", expect old TS " + oldTS + ", but found " + realLastModify); } } putResourceImpl(resPath, new ByteArrayInputStream(content), newTS); diff --git a/core-common/src/main/java/org/apache/kylin/common/persistence/ResourceStore.java b/core-common/src/main/java/org/apache/kylin/common/persistence/ResourceStore.java index 2bccd67..bda6cd0 100644 --- a/core-common/src/main/java/org/apache/kylin/common/persistence/ResourceStore.java +++ b/core-common/src/main/java/org/apache/kylin/common/persistence/ResourceStore.java @@ -276,14 +276,16 @@ abstract public class ResourceStore { /** * check & set, overwrite a resource */ - final public <T extends RootPersistentEntity> long putResource(String resPath, T obj, Serializer<T> serializer) throws IOException { + final public <T extends RootPersistentEntity> long putResource(String resPath, T obj, Serializer<T> serializer) + throws IOException, WriteConflictException { return putResource(resPath, obj, System.currentTimeMillis(), serializer); } /** * check & set, overwrite a resource */ - final public <T extends RootPersistentEntity> long putResource(String resPath, T obj, long newTS, Serializer<T> serializer) throws IOException { + final public <T extends RootPersistentEntity> long putResource(String resPath, T obj, long newTS, + Serializer<T> serializer) throws IOException, WriteConflictException { resPath = norm(resPath); //logger.debug("Saving resource " + resPath + " (Store " + kylinConfig.getMetadataUrl() + ")"); @@ -309,7 +311,8 @@ abstract public class ResourceStore { } } - private long checkAndPutResourceCheckpoint(String resPath, byte[] content, long oldTS, long newTS) throws IOException { + private long checkAndPutResourceCheckpoint(String resPath, byte[] content, long oldTS, long newTS) + throws IOException, WriteConflictException { beforeChange(resPath); return checkAndPutResourceImpl(resPath, content, oldTS, newTS); } @@ -317,7 +320,8 @@ abstract public class ResourceStore { /** * checks old timestamp when overwriting existing */ - abstract protected long checkAndPutResourceImpl(String resPath, byte[] content, long oldTS, long newTS) throws IOException, IllegalStateException; + abstract protected long checkAndPutResourceImpl(String resPath, byte[] content, long oldTS, long newTS) + throws IOException, WriteConflictException; /** * delete a resource, does nothing on a folder diff --git a/core-common/src/main/java/org/apache/kylin/common/persistence/StorageException.java b/core-common/src/main/java/org/apache/kylin/common/persistence/WriteConflictException.java similarity index 84% rename from core-common/src/main/java/org/apache/kylin/common/persistence/StorageException.java rename to core-common/src/main/java/org/apache/kylin/common/persistence/WriteConflictException.java index 604941a..f235b05 100644 --- a/core-common/src/main/java/org/apache/kylin/common/persistence/StorageException.java +++ b/core-common/src/main/java/org/apache/kylin/common/persistence/WriteConflictException.java @@ -19,19 +19,16 @@ package org.apache.kylin.common.persistence; /** - * - * @author xjiang - * */ -public class StorageException extends RuntimeException { +public class WriteConflictException extends RuntimeException { private static final long serialVersionUID = -3748712888242406257L; - public StorageException(String msg, Throwable t) { + public WriteConflictException(String msg, Throwable t) { super(msg, t); } - public StorageException(String msg) { + public WriteConflictException(String msg) { super(msg); } diff --git a/core-common/src/test/java/org/apache/kylin/common/persistence/ResourceStoreTest.java b/core-common/src/test/java/org/apache/kylin/common/persistence/ResourceStoreTest.java index f183e7c..3e1a31c 100644 --- a/core-common/src/test/java/org/apache/kylin/common/persistence/ResourceStoreTest.java +++ b/core-common/src/test/java/org/apache/kylin/common/persistence/ResourceStoreTest.java @@ -137,7 +137,7 @@ public class ResourceStoreTest { t.setLastModified(t.getLastModified() - 1); store.putResource(path2, t, StringEntity.serializer); fail("write conflict should trigger IllegalStateException"); - } catch (IllegalStateException e) { + } catch (WriteConflictException e) { // expected } diff --git a/core-cube/src/main/java/org/apache/kylin/cube/CubeManager.java b/core-cube/src/main/java/org/apache/kylin/cube/CubeManager.java index 15bb676..b8deadb 100755 --- a/core-cube/src/main/java/org/apache/kylin/cube/CubeManager.java +++ b/core-cube/src/main/java/org/apache/kylin/cube/CubeManager.java @@ -37,6 +37,7 @@ import org.apache.kylin.common.KylinConfig; import org.apache.kylin.common.persistence.JsonSerializer; import org.apache.kylin.common.persistence.ResourceStore; import org.apache.kylin.common.persistence.Serializer; +import org.apache.kylin.common.persistence.WriteConflictException; import org.apache.kylin.common.util.AutoReadWriteLock; import org.apache.kylin.common.util.AutoReadWriteLock.AutoLock; import org.apache.kylin.common.util.Dictionary; @@ -354,7 +355,7 @@ public class CubeManager implements IRealizationProvider { try { cube = crud.save(cube); - } catch (IllegalStateException ise) { + } catch (WriteConflictException ise) { logger.warn("Write conflict to update cube " + cube.getName() + " at try " + retry + ", will retry..."); if (retry >= 7) { logger.error("Retried 7 times till got error, abandoning...", ise); diff --git a/core-metadata/src/main/java/org/apache/kylin/metadata/TempStatementManager.java b/core-metadata/src/main/java/org/apache/kylin/metadata/TempStatementManager.java index 970df0c..182fc47 100644 --- a/core-metadata/src/main/java/org/apache/kylin/metadata/TempStatementManager.java +++ b/core-metadata/src/main/java/org/apache/kylin/metadata/TempStatementManager.java @@ -25,6 +25,7 @@ import java.util.List; import org.apache.kylin.common.KylinConfig; import org.apache.kylin.common.persistence.ResourceStore; import org.apache.kylin.common.persistence.RootPersistentEntity; +import org.apache.kylin.common.persistence.WriteConflictException; import org.apache.kylin.common.util.AutoReadWriteLock; import org.apache.kylin.common.util.AutoReadWriteLock.AutoLock; import org.apache.kylin.metadata.cachesync.Broadcaster; @@ -136,7 +137,7 @@ public class TempStatementManager { private void updateTempStatementWithRetry(TempStatementEntity entity, int retry) throws IOException { try { crud.save(entity); - } catch (IllegalStateException ise) { + } catch (WriteConflictException ise) { logger.warn("Write conflict to update temp statement" + entity.statementId + " at try " + retry + ", will retry..."); if (retry >= 7) { diff --git a/server-base/src/main/java/org/apache/kylin/rest/service/AclService.java b/server-base/src/main/java/org/apache/kylin/rest/service/AclService.java index f3e2393..4f439fe 100644 --- a/server-base/src/main/java/org/apache/kylin/rest/service/AclService.java +++ b/server-base/src/main/java/org/apache/kylin/rest/service/AclService.java @@ -32,6 +32,7 @@ import org.apache.kylin.common.KylinConfig; import org.apache.kylin.common.persistence.JsonSerializer; import org.apache.kylin.common.persistence.ResourceStore; import org.apache.kylin.common.persistence.Serializer; +import org.apache.kylin.common.persistence.WriteConflictException; import org.apache.kylin.common.util.AutoReadWriteLock; import org.apache.kylin.common.util.AutoReadWriteLock.AutoLock; import org.apache.kylin.metadata.cachesync.Broadcaster; @@ -305,7 +306,7 @@ public class AclService implements MutableAclService, InitializingBean { crud.save(record); return acl; // here we are done - } catch (IllegalStateException ise) { + } catch (WriteConflictException ise) { if (retry <= 0) { logger.error("Retry is out, till got error, abandoning...", ise); throw ise; diff --git a/storage-hbase/src/main/java/org/apache/kylin/storage/hbase/HBaseConnection.java b/storage-hbase/src/main/java/org/apache/kylin/storage/hbase/HBaseConnection.java index 0c2fb04..53e8a68 100644 --- a/storage-hbase/src/main/java/org/apache/kylin/storage/hbase/HBaseConnection.java +++ b/storage-hbase/src/main/java/org/apache/kylin/storage/hbase/HBaseConnection.java @@ -49,7 +49,6 @@ import org.apache.hadoop.hdfs.DFSConfigKeys; import org.apache.kylin.common.KylinConfig; import org.apache.kylin.common.StorageURL; import org.apache.kylin.common.lock.DistributedLock; -import org.apache.kylin.common.persistence.StorageException; import org.apache.kylin.common.util.HadoopUtil; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -269,7 +268,7 @@ public class HBaseConnection { } catch (Throwable t) { logger.error("Error when open connection " + url, t); - throw new StorageException("Error when open connection " + url, t); + throw new RuntimeException("Error when open connection " + url, t); } return connection; diff --git a/storage-hbase/src/main/java/org/apache/kylin/storage/hbase/HBaseResourceStore.java b/storage-hbase/src/main/java/org/apache/kylin/storage/hbase/HBaseResourceStore.java index 3762437..23df556 100644 --- a/storage-hbase/src/main/java/org/apache/kylin/storage/hbase/HBaseResourceStore.java +++ b/storage-hbase/src/main/java/org/apache/kylin/storage/hbase/HBaseResourceStore.java @@ -54,6 +54,7 @@ import org.apache.kylin.common.StorageURL; import org.apache.kylin.common.persistence.RawResource; import org.apache.kylin.common.persistence.ResourceStore; import org.apache.kylin.common.persistence.StringEntity; +import org.apache.kylin.common.persistence.WriteConflictException; import org.apache.kylin.common.util.Bytes; import org.apache.kylin.common.util.BytesUtil; import org.apache.kylin.common.util.HadoopUtil; @@ -321,7 +322,7 @@ public class HBaseResourceStore extends ResourceStore { + ", operation result: " + ok); if (!ok) { long real = getResourceTimestampImpl(resPath); - throw new IllegalStateException( + throw new WriteConflictException( "Overwriting conflict " + resPath + ", expect old TS " + oldTS + ", but it is " + real); } -- To stop receiving notification emails like this one, please contact liy...@apache.org.