You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@kylin.apache.org by ni...@apache.org on 2019/06/25 07:36:34 UTC
[kylin] branch 2.6.x updated: #KYLIN-4030,
Resource deleteResource by comparing timestamp may failed to delete
caused by time precision loose
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 <co...@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);
}