This is an automated email from the ASF dual-hosted git repository.
nic pushed a commit to branch 2.6.x
in repository https://gitbox.apache.org/repos/asf/kylin.git
The following commit(s) were added to refs/heads/2.6.x by this push:
new 414e7a1 #KYLIN-4030, Resource deleteResource by comparing timestamp
may failed to delete caused by time precision loose
414e7a1 is described below
commit 414e7a18adf3992df2f311c0e3d0de9436ca9587
Author: ZhengshuaiPENG <cosine...@hotmail.com>
AuthorDate: Mon Jun 3 20:26:34 2019 +0800
#KYLIN-4030, Resource deleteResource by comparing timestamp may failed to
delete caused by time precision loose
---
.../kylin/common/persistence/FileResourceStore.java | 6 +++++-
.../kylin/common/persistence/HDFSResourceStore.java | 7 ++++++-
.../kylin/common/persistence/JDBCResourceStore.java | 6 +++++-
.../apache/kylin/common/persistence/ResourceStore.java | 17 ++++++++++++++++-
.../kylin/common/persistence/ResourceStoreTest.java | 10 +++++++---
.../apache/kylin/storage/hbase/HBaseResourceStore.java | 4 ++++
6 files changed, 43 insertions(+), 7 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 813eca3..75c625b 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
@@ -208,8 +208,12 @@ public class FileResourceStore extends ResourceStore {
try {
if (f.exists()) {
long origLastModified = getResourceTimestampImpl(resPath);
- if (checkTimeStampBeforeDelete(origLastModified, timestamp))
+ if (checkTimeStampBeforeDelete(origLastModified, timestamp)) {
FileUtils.forceDelete(f);
+ } else {
+ throw new IOException("Resource " + resPath + " timestamp
not match, [originLastModified: "
+ + origLastModified + ", timestampToDelete: " +
timestamp + "]");
+ }
}
} catch (FileNotFoundException e) {
// FileNotFoundException is not a problem in case of delete
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 03cab1f..b89338b 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
@@ -277,8 +277,13 @@ public class HDFSResourceStore extends ResourceStore {
Path p = getRealHDFSPath(resPath);
if (fs.exists(p)) {
long origLastModified =
fs.getFileStatus(p).getModificationTime();
- if (checkTimeStampBeforeDelete(origLastModified, timestamp))
+ if (checkTimeStampBeforeDelete(origLastModified, timestamp)) {
fs.delete(p, true);
+ } else {
+ throw new IOException("Resource " + resPath + " timestamp
not match, [originLastModified: "
+ + origLastModified + ", timestampToDelete: " +
timestamp + "]");
+ }
+
}
} catch (Exception e) {
throw new IOException("Delete resource fail", e);
diff --git
a/core-common/src/main/java/org/apache/kylin/common/persistence/JDBCResourceStore.java
b/core-common/src/main/java/org/apache/kylin/common/persistence/JDBCResourceStore.java
index 0bb8f4c..2874bbd 100644
---
a/core-common/src/main/java/org/apache/kylin/common/persistence/JDBCResourceStore.java
+++
b/core-common/src/main/java/org/apache/kylin/common/persistence/JDBCResourceStore.java
@@ -573,8 +573,12 @@ public class JDBCResourceStore extends
PushdownResourceStore {
protected void deleteResourceImpl(String resPath, long timestamp) throws
IOException {
// considering deletePushDown operation, check timestamp at the
beginning
long origLastModified = getResourceTimestampImpl(resPath);
- if (checkTimeStampBeforeDelete(origLastModified, timestamp))
+ if (checkTimeStampBeforeDelete(origLastModified, timestamp)) {
deleteResourceImpl(resPath);
+ } else {
+ throw new IOException("Resource " + resPath + " timestamp not
match, [originLastModified: "
+ + origLastModified + ", timestampToDelete: " + timestamp +
"]");
+ }
}
@Override
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 57a6ef0..3eeeb69 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
@@ -476,6 +476,12 @@ abstract public class ResourceStore {
deleteResourceCheckpoint(norm(resPath));
}
+ /**
+ * Delete a resource with comparing its timestamp
+ * Success to delete if resource lastModified < timestamp + 1000
(considering timestamp precision loose)
+ * throw an IOException when the resource lastModified >= timestamp + 1000
+ * See https://issues.apache.org/jira/browse/KYLIN-4030
+ */
final public void deleteResource(String resPath, long timestamp) throws
IOException {
logger.trace("Deleting resource {} within timestamp {} (Store {})",
resPath, timestamp,
kylinConfig.getMetadataUrl());
@@ -521,7 +527,16 @@ abstract public class ResourceStore {
protected boolean checkTimeStampBeforeDelete(long originLastModified, long
timestamp) {
// note here is originLastModified may be 0
// 0 means resource doesn't exists in general, it's safe to pass the
check
- boolean passCheck = originLastModified <= timestamp;
+ boolean passCheck = false;
+ if (originLastModified > timestamp) {
+ // file system may loose time precision with milliseconds
+ // because of the new born resource time, so here if time diff
less than 1000 ms, we will treat it the same
+ long timeDiff = originLastModified - timestamp;
+ passCheck = timeDiff < 1000;
+ } else {
+ // if timestamp >= originLastModified, it's safe to delete
+ passCheck = true;
+ }
logger.trace("check timestamp before delete: {}, [originLastModified:
{}, timestamp: {}]", passCheck,
originLastModified, timestamp);
return passCheck;
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 8cd181a..3e03c3f 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
@@ -208,10 +208,14 @@ public class ResourceStoreTest {
assertTrue(list == null || !list.contains(path2));
long origLastModified = store.getResourceTimestamp(path3);
- long beforeLastModified = origLastModified - 100;
+ long beforeLastModified = origLastModified - 1000;
// beforeLastModified < origLastModified ==> not delete expected
- store.deleteResource(path3, beforeLastModified);
+ try {
+ store.deleteResource(path3, beforeLastModified);
+ } catch (Exception e) {
+ assertTrue(e instanceof IOException);
+ }
assertTrue(store.exists(path3));
list = store.listResources(dir3);
assertTrue(list != null && list.contains(path3));
@@ -230,7 +234,7 @@ public class ResourceStoreTest {
assertEquals(content3, t);
origLastModified = store.getResourceTimestamp(path3);
- long afterLastModified = origLastModified + 100;
+ long afterLastModified = origLastModified + 1000;
// afterLastModified > origLastModified ==> delete expected
store.deleteResource(path3, afterLastModified);
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 ecd698f..f1960a3 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
@@ -411,7 +411,11 @@ public class HBaseResourceStore extends
PushdownResourceStore {
if (hdfsResourceExist) { // remove hdfs cell value
deletePushdown(resPath);
}
+ } else {
+ throw new IOException("Resource " + resPath + " timestamp not
match, [originLastModified: "
+ + origLastModified + ", timestampToDelete: " +
timestamp + "]");
}
+
} finally {
IOUtils.closeQuietly(table);
}
