You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@zeppelin.apache.org by jo...@apache.org on 2017/01/24 06:31:39 UTC
zeppelin git commit: [ZEPPELIN-1921] missing close in
closeAndRemoveInterpreterGroup method
Repository: zeppelin
Updated Branches:
refs/heads/master 92cebe59d -> 3bbcf0969
[ZEPPELIN-1921] missing close in closeAndRemoveInterpreterGroup method
### What is this PR for?
The problem is that some code in the closeAndRemoveInterpreterGroup method of InterpreterSetting partially closes.
### What type of PR is it?
Bug Fix
### What is the Jira issue?
https://issues.apache.org/jira/browse/ZEPPELIN-1921
### Questions:
* Does the licenses files need update? no
* Is there breaking changes for older versions? no
* Does this needs documentation? no
Author: cloverhearts <cl...@gmail.com>
Closes #1864 from cloverhearts/ZEPPELIN-1921 and squashes the following commits:
4ac76cf [cloverhearts] Merge branch 'master' into ZEPPELIN-1921
f39212c [cloverhearts] fixed equals logic
e21287f [cloverhearts] apply other side
6b86dfd [cloverhearts] compare logic change
6a2051c [cloverhearts] method name change (master rebase)
d12ec57 [cloverhearts] missing brace
ca9ecfd [cloverhearts] Merge branch 'master' into ZEPPELIN-1921
22473a2 [cloverhearts] change return logic
a105adf [cloverhearts] Merge branch 'master' into ZEPPELIN-1921
b0a9396 [cloverhearts] test case and replace logic
2482be6 [cloverhearts] container method -> isEqualInterpreterKey method
e25f311 [cloverhearts] interpreter test case and replace logic
546ee85 [cloverhearts] Revert "change Linkedlist to LinkedHashSet"
59c9c76 [cloverhearts] implement testcase
2188b1b [cloverhearts] change Linkedlist to LinkedHashSet
0ebed44 [cloverhearts] fixed missing for close
Project: http://git-wip-us.apache.org/repos/asf/zeppelin/repo
Commit: http://git-wip-us.apache.org/repos/asf/zeppelin/commit/3bbcf096
Tree: http://git-wip-us.apache.org/repos/asf/zeppelin/tree/3bbcf096
Diff: http://git-wip-us.apache.org/repos/asf/zeppelin/diff/3bbcf096
Branch: refs/heads/master
Commit: 3bbcf0969749bfcfc3cb0a58a3af295a44959bd8
Parents: 92cebe5
Author: cloverhearts <cl...@gmail.com>
Authored: Fri Jan 20 13:38:17 2017 -0800
Committer: Jongyoul Lee <jo...@apache.org>
Committed: Tue Jan 24 15:31:31 2017 +0900
----------------------------------------------------------------------
.../interpreter/InterpreterSetting.java | 55 +++++++++++++++-----
.../interpreter/mock/MockInterpreter11.java | 7 +++
.../notebook/NoteInterpreterLoaderTest.java | 55 ++++++++++++++++++++
3 files changed, 105 insertions(+), 12 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/zeppelin/blob/3bbcf096/zeppelin-zengine/src/main/java/org/apache/zeppelin/interpreter/InterpreterSetting.java
----------------------------------------------------------------------
diff --git a/zeppelin-zengine/src/main/java/org/apache/zeppelin/interpreter/InterpreterSetting.java b/zeppelin-zengine/src/main/java/org/apache/zeppelin/interpreter/InterpreterSetting.java
index 9176ddf..bd7d664 100644
--- a/zeppelin-zengine/src/main/java/org/apache/zeppelin/interpreter/InterpreterSetting.java
+++ b/zeppelin-zengine/src/main/java/org/apache/zeppelin/interpreter/InterpreterSetting.java
@@ -17,6 +17,7 @@
package org.apache.zeppelin.interpreter;
+import java.util.Arrays;
import java.util.Collection;
import java.util.HashMap;
import java.util.HashSet;
@@ -144,6 +145,33 @@ public class InterpreterSetting {
return key;
}
+ private boolean isEqualInterpreterKeyProcessKey(String refKey, String processKey) {
+ InterpreterOption option = getOption();
+ int validCount = 0;
+ if (getOption().isProcess()
+ && !(option.perUserIsolated() == true && option.perNoteIsolated() == true)) {
+
+ List<String> processList = Arrays.asList(processKey.split(":"));
+ List<String> refList = Arrays.asList(refKey.split(":"));
+
+ if (refList.size() <= 1 || processList.size() <= 1) {
+ return refKey.equals(processKey);
+ }
+
+ if (processList.get(0).equals("") || processList.get(0).equals(refList.get(0))) {
+ validCount = validCount + 1;
+ }
+
+ if (processList.get(1).equals("") || processList.get(1).equals(refList.get(1))) {
+ validCount = validCount + 1;
+ }
+
+ return (validCount >= 2);
+ } else {
+ return refKey.equals(processKey);
+ }
+ }
+
private String getInterpreterSessionKey(String user, String noteId) {
InterpreterOption option = getOption();
String key;
@@ -194,18 +222,19 @@ public class InterpreterSetting {
}
void closeAndRemoveInterpreterGroupByNoteId(String noteId) {
- String key = getInterpreterProcessKey("", noteId);
-
- InterpreterGroup groupToRemove = null;
+ String processKey = getInterpreterProcessKey("", noteId);
+ List<InterpreterGroup> closeToGroupList = new LinkedList<>();
+ InterpreterGroup groupKey;
for (String intpKey : new HashSet<>(interpreterGroupRef.keySet())) {
- if (intpKey.contains(key)) {
+ if (isEqualInterpreterKeyProcessKey(intpKey, processKey)) {
interpreterGroupWriteLock.lock();
- groupToRemove = interpreterGroupRef.remove(intpKey);
+ groupKey = interpreterGroupRef.remove(intpKey);
interpreterGroupWriteLock.unlock();
+ closeToGroupList.add(groupKey);
}
}
- if (groupToRemove != null) {
+ for (InterpreterGroup groupToRemove : closeToGroupList) {
groupToRemove.close();
}
}
@@ -216,17 +245,19 @@ public class InterpreterSetting {
}
String processKey = getInterpreterProcessKey(user, "");
String sessionKey = getInterpreterSessionKey(user, "");
- InterpreterGroup groupToRemove = null;
+ List<InterpreterGroup> groupToRemove = new LinkedList<>();
+ InterpreterGroup groupItem;
for (String intpKey : new HashSet<>(interpreterGroupRef.keySet())) {
- if (intpKey.contains(processKey)) {
+ if (isEqualInterpreterKeyProcessKey(intpKey, processKey)) {
interpreterGroupWriteLock.lock();
- groupToRemove = interpreterGroupRef.remove(intpKey);
+ groupItem = interpreterGroupRef.remove(intpKey);
interpreterGroupWriteLock.unlock();
+ groupToRemove.add(groupItem);
}
}
- if (groupToRemove != null) {
- groupToRemove.close(sessionKey);
+ for (InterpreterGroup groupToClose : groupToRemove) {
+ groupToClose.close(sessionKey);
}
}
@@ -243,7 +274,7 @@ public class InterpreterSetting {
List<InterpreterGroup> groupToRemove = new LinkedList<>();
InterpreterGroup groupItem;
for (String intpKey : new HashSet<>(interpreterGroupRef.keySet())) {
- if (intpKey.contains(key)) {
+ if (isEqualInterpreterKeyProcessKey(intpKey, key)) {
interpreterGroupWriteLock.lock();
groupItem = interpreterGroupRef.remove(intpKey);
interpreterGroupWriteLock.unlock();
http://git-wip-us.apache.org/repos/asf/zeppelin/blob/3bbcf096/zeppelin-zengine/src/test/java/org/apache/zeppelin/interpreter/mock/MockInterpreter11.java
----------------------------------------------------------------------
diff --git a/zeppelin-zengine/src/test/java/org/apache/zeppelin/interpreter/mock/MockInterpreter11.java b/zeppelin-zengine/src/test/java/org/apache/zeppelin/interpreter/mock/MockInterpreter11.java
index fc30726..58200d8 100644
--- a/zeppelin-zengine/src/test/java/org/apache/zeppelin/interpreter/mock/MockInterpreter11.java
+++ b/zeppelin-zengine/src/test/java/org/apache/zeppelin/interpreter/mock/MockInterpreter11.java
@@ -35,13 +35,20 @@ public class MockInterpreter11 extends Interpreter{
public MockInterpreter11(Properties property) {
super(property);
}
+ boolean open;
@Override
public void open() {
+ open = true;
}
@Override
public void close() {
+ open = false;
+ }
+
+ public boolean isOpen() {
+ return open;
}
@Override
http://git-wip-us.apache.org/repos/asf/zeppelin/blob/3bbcf096/zeppelin-zengine/src/test/java/org/apache/zeppelin/notebook/NoteInterpreterLoaderTest.java
----------------------------------------------------------------------
diff --git a/zeppelin-zengine/src/test/java/org/apache/zeppelin/notebook/NoteInterpreterLoaderTest.java b/zeppelin-zengine/src/test/java/org/apache/zeppelin/notebook/NoteInterpreterLoaderTest.java
index 22e2039..320a5b5 100644
--- a/zeppelin-zengine/src/test/java/org/apache/zeppelin/notebook/NoteInterpreterLoaderTest.java
+++ b/zeppelin-zengine/src/test/java/org/apache/zeppelin/notebook/NoteInterpreterLoaderTest.java
@@ -20,6 +20,7 @@ import java.io.File;
import java.io.IOException;
import java.util.Collections;
import java.util.HashMap;
+import java.util.List;
import org.apache.zeppelin.conf.ZeppelinConfiguration;
import org.apache.zeppelin.conf.ZeppelinConfiguration.ConfVars;
@@ -28,13 +29,17 @@ import org.apache.zeppelin.interpreter.Interpreter;
import org.apache.zeppelin.interpreter.InterpreterFactory;
import org.apache.zeppelin.interpreter.InterpreterOption;
import org.apache.zeppelin.interpreter.InterpreterSetting;
+import org.apache.zeppelin.interpreter.LazyOpenInterpreter;
import org.apache.zeppelin.interpreter.mock.MockInterpreter1;
import org.apache.zeppelin.interpreter.mock.MockInterpreter11;
import org.apache.zeppelin.interpreter.mock.MockInterpreter2;
+import org.apache.zeppelin.interpreter.remote.RemoteInterpreter;
+import org.apache.zeppelin.interpreter.remote.RemoteInterpreterProcess;
import org.junit.After;
import org.junit.Before;
import org.junit.Test;
+import static java.lang.Thread.sleep;
import static org.junit.Assert.*;
public class NoteInterpreterLoaderTest {
@@ -117,6 +122,11 @@ public class NoteInterpreterLoaderTest {
assertNotNull(factory.getInterpreterSettings("noteA").get(0).getInterpreterGroup("user", "noteA").get("noteA"));
assertNotNull(factory.getInterpreterSettings("noteB").get(0).getInterpreterGroup("user", "noteB").get("noteB"));
+ // invalid close
+ factory.closeNote("user", "note");
+ assertNotNull(factory.getInterpreterSettings("noteA").get(0).getInterpreterGroup("user", "shared_process").get("noteA"));
+ assertNotNull(factory.getInterpreterSettings("noteB").get(0).getInterpreterGroup("user", "shared_process").get("noteB"));
+
// when
factory.closeNote("user", "noteA");
factory.closeNote("user", "noteB");
@@ -160,6 +170,51 @@ public class NoteInterpreterLoaderTest {
assertNull(factory.getInterpreterSettings("noteB").get(0).getInterpreterGroup("user", "noteB").get("shared_session"));
}
+ @Test
+ public void testNoteInterpreterCloseForAll() throws IOException {
+ factory.setInterpreters("user", "FitstNote", factory.getDefaultInterpreterSettingList());
+ factory.getInterpreterSettings("FitstNote").get(0).getOption().setPerNote(InterpreterOption.SCOPED);
+
+ factory.setInterpreters("user", "yourFirstNote", factory.getDefaultInterpreterSettingList());
+ factory.getInterpreterSettings("yourFirstNote").get(0).getOption().setPerNote(InterpreterOption.ISOLATED);
+
+ // interpreters are not created before accessing it
+ assertNull(factory.getInterpreterSettings("FitstNote").get(0).getInterpreterGroup("user", "FitstNote").get("FitstNote"));
+ assertNull(factory.getInterpreterSettings("yourFirstNote").get(0).getInterpreterGroup("user", "yourFirstNote").get("yourFirstNote"));
+
+ Interpreter firstNoteIntp = factory.getInterpreter("user", "FitstNote", "group1.mock1");
+ Interpreter yourFirstNoteIntp = factory.getInterpreter("user", "yourFirstNote", "group1.mock1");
+
+ firstNoteIntp.open();
+ yourFirstNoteIntp.open();
+
+ assertTrue(((LazyOpenInterpreter)firstNoteIntp).isOpen());
+ assertTrue(((LazyOpenInterpreter)yourFirstNoteIntp).isOpen());
+
+ factory.closeNote("user", "FitstNote");
+
+ assertFalse(((LazyOpenInterpreter)firstNoteIntp).isOpen());
+ assertTrue(((LazyOpenInterpreter)yourFirstNoteIntp).isOpen());
+
+ //reopen
+ firstNoteIntp.open();
+
+ assertTrue(((LazyOpenInterpreter)firstNoteIntp).isOpen());
+ assertTrue(((LazyOpenInterpreter)yourFirstNoteIntp).isOpen());
+
+ // invalid check
+ factory.closeNote("invalid", "Note");
+
+ assertTrue(((LazyOpenInterpreter)firstNoteIntp).isOpen());
+ assertTrue(((LazyOpenInterpreter)yourFirstNoteIntp).isOpen());
+
+ // invalid contains value check
+ factory.closeNote("u", "Note");
+
+ assertTrue(((LazyOpenInterpreter)firstNoteIntp).isOpen());
+ assertTrue(((LazyOpenInterpreter)yourFirstNoteIntp).isOpen());
+ }
+
private void delete(File file){
if(file.isFile()) file.delete();