[
https://issues.apache.org/jira/browse/KYLIN-3633?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16660643#comment-16660643
]
ASF GitHub Bot commented on KYLIN-3633:
---------------------------------------
shaofengshi closed pull request #301: KYLIN-3633 Avoid potential dead lock when
building global dictionary
URL: https://github.com/apache/kylin/pull/301
This is a PR merged from a forked repository.
As GitHub hides the original diff on merge, it is displayed below for
the sake of provenance:
As this is a foreign pull request (from a fork), the diff is supplied
below (as it won't show otherwise due to GitHub magic):
diff --git
a/core-dictionary/src/main/java/org/apache/kylin/dict/DictionaryGenerator.java
b/core-dictionary/src/main/java/org/apache/kylin/dict/DictionaryGenerator.java
index db0c302970..7c33b4a1e4 100644
---
a/core-dictionary/src/main/java/org/apache/kylin/dict/DictionaryGenerator.java
+++
b/core-dictionary/src/main/java/org/apache/kylin/dict/DictionaryGenerator.java
@@ -75,13 +75,19 @@ public static IDictionaryBuilder
newDictionaryBuilder(DataType dataType) {
builder.init(dictInfo, baseId, null);
// add values
- while (valueEnumerator.moveNext()) {
- String value = valueEnumerator.current();
+ try {
+ while (valueEnumerator.moveNext()) {
+ String value = valueEnumerator.current();
- boolean accept = builder.addValue(value);
+ boolean accept = builder.addValue(value);
- if (accept && samples.size() < nSamples && samples.contains(value)
== false)
- samples.add(value);
+ if (accept && samples.size() < nSamples &&
samples.contains(value) == false)
+ samples.add(value);
+ }
+ } catch (IOException e) {
+ logger.error("Error during adding dict value.", e);
+ builder.clear();
+ throw e;
}
// build
@@ -149,6 +155,12 @@ public boolean addValue(String value) {
return new DateStrDictionary(datePattern, baseId);
}
+
+
+ @Override
+ public void clear() {
+ // do nothing
+ }
}
private static class TimeDictBuilder implements IDictionaryBuilder {
@@ -171,6 +183,11 @@ public boolean addValue(String value) {
public Dictionary<String> build() throws IOException {
return new TimeStrDictionary(); // base ID is always 0
}
+
+ @Override
+ public void clear() {
+
+ }
}
private static class StringTrieDictBuilder implements IDictionaryBuilder {
@@ -196,6 +213,11 @@ public boolean addValue(String value) {
public Dictionary<String> build() throws IOException {
return builder.build(baseId);
}
+
+ @Override
+ public void clear() {
+
+ }
}
private static class StringTrieDictForestBuilder implements
IDictionaryBuilder {
@@ -219,6 +241,11 @@ public boolean addValue(String value) {
public Dictionary<String> build() throws IOException {
return builder.build();
}
+
+ @Override
+ public void clear() {
+
+ }
}
@SuppressWarnings("deprecation")
@@ -245,6 +272,11 @@ public boolean addValue(String value) {
public Dictionary<String> build() throws IOException {
return builder.build(baseId);
}
+
+ @Override
+ public void clear() {
+
+ }
}
private static class NumberTrieDictForestBuilder implements
IDictionaryBuilder {
@@ -268,6 +300,11 @@ public boolean addValue(String value) {
public Dictionary<String> build() throws IOException {
return builder.build();
}
+
+ @Override
+ public void clear() {
+
+ }
}
}
diff --git
a/core-dictionary/src/main/java/org/apache/kylin/dict/GlobalDictionaryBuilder.java
b/core-dictionary/src/main/java/org/apache/kylin/dict/GlobalDictionaryBuilder.java
index 9168ca4a1a..d813793ea1 100644
---
a/core-dictionary/src/main/java/org/apache/kylin/dict/GlobalDictionaryBuilder.java
+++
b/core-dictionary/src/main/java/org/apache/kylin/dict/GlobalDictionaryBuilder.java
@@ -19,8 +19,8 @@
package org.apache.kylin.dict;
import java.io.IOException;
-
import java.util.Locale;
+
import org.apache.kylin.common.KylinConfig;
import org.apache.kylin.common.lock.DistributedLock;
import org.apache.kylin.common.util.Dictionary;
@@ -104,6 +104,13 @@ public boolean addValue(String value) {
return new AppendTrieDictionary<>();
}
+ @Override
+ public void clear() {
+ if (lock.isLocked(getLockPath(sourceColumn))) {
+ lock.unlock(getLockPath(sourceColumn));
+ }
+ }
+
private String getLockPath(String pathName) {
return "/dict/" + pathName + "/lock";
}
diff --git
a/core-dictionary/src/main/java/org/apache/kylin/dict/IDictionaryBuilder.java
b/core-dictionary/src/main/java/org/apache/kylin/dict/IDictionaryBuilder.java
index e2a643dbd0..771bfb42c6 100644
---
a/core-dictionary/src/main/java/org/apache/kylin/dict/IDictionaryBuilder.java
+++
b/core-dictionary/src/main/java/org/apache/kylin/dict/IDictionaryBuilder.java
@@ -35,4 +35,7 @@
/** Build the dictionary */
Dictionary<String> build() throws IOException;
+
+ /** Clear before exit */
+ void clear();
}
diff --git
a/core-dictionary/src/main/java/org/apache/kylin/dict/global/SegmentAppendTrieDictBuilder.java
b/core-dictionary/src/main/java/org/apache/kylin/dict/global/SegmentAppendTrieDictBuilder.java
index f8640a0b2a..770b0bc193 100644
---
a/core-dictionary/src/main/java/org/apache/kylin/dict/global/SegmentAppendTrieDictBuilder.java
+++
b/core-dictionary/src/main/java/org/apache/kylin/dict/global/SegmentAppendTrieDictBuilder.java
@@ -78,4 +78,9 @@ public boolean addValue(String value) {
public Dictionary<String> build() throws IOException {
return builder.build(baseId);
}
+
+ @Override
+ public void clear() {
+
+ }
}
diff --git
a/kylin-it/src/test/java/org/apache/kylin/dict/ITGlobalDictionaryBuilderTest.java
b/kylin-it/src/test/java/org/apache/kylin/dict/ITGlobalDictionaryBuilderTest.java
index c578a57c34..94c4f56640 100644
---
a/kylin-it/src/test/java/org/apache/kylin/dict/ITGlobalDictionaryBuilderTest.java
+++
b/kylin-it/src/test/java/org/apache/kylin/dict/ITGlobalDictionaryBuilderTest.java
@@ -18,20 +18,21 @@
package org.apache.kylin.dict;
+import java.io.IOException;
+import java.util.concurrent.CountDownLatch;
+
import org.apache.hadoop.fs.Path;
import org.apache.kylin.common.KylinConfig;
+import org.apache.kylin.common.lock.DistributedLock;
import org.apache.kylin.common.util.Dictionary;
import org.apache.kylin.common.util.HBaseMetadataTestCase;
import org.apache.kylin.common.util.HadoopUtil;
import org.junit.After;
+import org.junit.Assert;
import org.junit.Before;
+import org.junit.Rule;
import org.junit.Test;
-
-import java.io.IOException;
-import java.util.concurrent.CountDownLatch;
-
-import static org.junit.Assert.assertEquals;
-import static org.junit.Assert.assertNotEquals;
+import org.junit.rules.ExpectedException;
public class ITGlobalDictionaryBuilderTest extends HBaseMetadataTestCase {
private DictionaryInfo dictionaryInfo;
@@ -48,8 +49,12 @@ public void afterTest() {
staticCleanupTestMetadata();
}
+ @Rule
+ public ExpectedException thrown = ExpectedException.none();
+
private void cleanup() {
- String BASE_DIR =
KylinConfig.getInstanceFromEnv().getHdfsWorkingDirectory() +
"/resources/GlobalDict" + dictionaryInfo.getResourceDir() + "/";
+ String BASE_DIR =
KylinConfig.getInstanceFromEnv().getHdfsWorkingDirectory() +
"/resources/GlobalDict"
+ + dictionaryInfo.getResourceDir() + "/";
Path basePath = new Path(BASE_DIR);
try {
HadoopUtil.getFileSystem(basePath).delete(basePath, true);
@@ -77,16 +82,33 @@ public void testGlobalDictLock() throws IOException,
InterruptedException {
Dictionary<String> dict = builder.build();
for (int i = 0; i < 10000; i++) {
- assertNotEquals(-1, dict.getIdFromValue("t1_" + i));
+ Assert.assertNotEquals(-1, dict.getIdFromValue("t1_" + i));
}
for (int i = 0; i < 10; i++) {
- assertNotEquals(-1, dict.getIdFromValue("t2_" + i));
+ Assert.assertNotEquals(-1, dict.getIdFromValue("t2_" + i));
}
for (int i = 0; i < 100000; i++) {
- assertNotEquals(-1, dict.getIdFromValue("t3_" + i));
+ Assert.assertNotEquals(-1, dict.getIdFromValue("t3_" + i));
}
- assertEquals(110011, dict.getIdFromValue("success"));
+ Assert.assertEquals(110011, dict.getIdFromValue("success"));
+ }
+
+ @Test
+ public void testBuildGlobalDictFailed() throws IOException {
+ thrown.expect(IOException.class);
+ thrown.expectMessage("read failed.");
+
+ GlobalDictionaryBuilder builder = new GlobalDictionaryBuilder();
+ try {
+ DictionaryGenerator.buildDictionary(builder, dictionaryInfo, new
ErrorDictionaryValueEnumerator());
+ } catch (Throwable e) {
+ DistributedLock lock =
KylinConfig.getInstanceFromEnv().getDistributedLockFactory().lockForCurrentThread();
+ String lockPath = "/dict/" + dictionaryInfo.getSourceTable() + "_"
+ dictionaryInfo.getSourceColumn()
+ + "/lock";
+ Assert.assertFalse(lock.isLocked(lockPath));
+ throw e;
+ }
}
private class SharedBuilderThread extends Thread {
@@ -118,4 +140,26 @@ public void run() {
}
}
}
-}
+
+ private class ErrorDictionaryValueEnumerator implements
IDictionaryValueEnumerator {
+ private int idx = 0;
+
+ @Override
+ public String current() throws IOException {
+ return null;
+ }
+
+ @Override
+ public boolean moveNext() throws IOException {
+ idx++;
+ if (idx == 1)
+ throw new IOException("read failed.");
+ return true;
+ }
+
+ @Override
+ public void close() throws IOException {
+
+ }
+ }
+}
\ No newline at end of file
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
> Dead lock may happen in building global dictionary
> --------------------------------------------------
>
> Key: KYLIN-3633
> URL: https://issues.apache.org/jira/browse/KYLIN-3633
> Project: Kylin
> Issue Type: Bug
> Components: Measure - Count Distinct
> Affects Versions: v2.4.1
> Reporter: nichunen
> Assignee: nichunen
> Priority: Major
> Fix For: v2.5.1
>
>
> During the building of global dict, if an error happens, the builder's lock
> maybe un-released. This will block all other building jobs with global dict.
--
This message was sent by Atlassian JIRA
(v7.6.3#76005)
