-
Notifications
You must be signed in to change notification settings - Fork 2.8k
Pool pluggable rebase #843
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Closed
Closed
Changes from all commits
Commits
Show all changes
18 commits
Select commit
Hold shift + click to select a range
43fa7b9
Added serialization to resourcepoolutils.
fireboy1919 5de5756
Added pool persistance.
fireboy1919 e633c44
Added test.
fireboy1919 71609fa
Changed resource pool utils so that it will work anywhere.
fireboy1919 6a22e3b
Removed unnecessary serialization.
fireboy1919 17fed3d
Added explicit save to the serializer.
fireboy1919 067da37
Added vfs resource pool.
fireboy1919 0cfeb33
Added pluggable resource pool.
fireboy1919 52cf1be
Added missing rename.
fireboy1919 a1b0bbd
Added a constructor to DistributedResourcePools to fix tests.
fireboy1919 f241272
Switched to java 7 method for java.util.properties.
fireboy1919 3ed89b0
Removed the need for resource pools during tests.
fireboy1919 0fa4b32
Adding missing license file.
fireboy1919 69d11d3
Set mock interpreter to register properly.
fireboy1919 df46376
Switched interpreter to use default configuration.
fireboy1919 1c29288
Merge remote-tracking branch 'apache/master' into pool_pluggable_rebase
fireboy1919 bbeacac
Fixed missing comma.
fireboy1919 3993c0d
Added fixes to tests
fireboy1919 File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -134,17 +134,48 @@ public static void main(String[] args) | |
| System.exit(0); | ||
| } | ||
|
|
||
|
|
||
| private DistributedResourcePool getResourcePool() | ||
| /* InterpreterGroup group, | ||
| Properties prop, | ||
| RemoteInterpreterEventClient client) */ | ||
| throws TException { | ||
| if (resourcePool != null) | ||
| return resourcePool; | ||
| try { | ||
| Properties prop = interpreterGroup.getProperty(); | ||
| //Happens during tests. | ||
| if (prop == null) | ||
| prop = new Properties(); | ||
| String resourcePoolClassName = (String) prop.getProperty( | ||
| "zeppelin.interpreter.resourcePoolClass"); | ||
| logger.debug("Getting resource pool {}", resourcePoolClassName); | ||
| Class resourcePoolClass = Class.forName(resourcePoolClassName); | ||
|
|
||
| Constructor<ResourcePool> constructor = resourcePoolClass | ||
| .getConstructor(new Class[] {String.class, | ||
| ResourcePoolConnector.class, | ||
| Properties.class }); | ||
| resourcePool = (DistributedResourcePool) constructor.newInstance(interpreterGroup.getId(), | ||
| this.eventClient, | ||
| prop); | ||
| } catch (Exception e) { | ||
| logger.error("Did not find resource pool. Using DistributedResourcePool"); | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. use logger.error("message", e) instead |
||
| resourcePool = new DistributedResourcePool(interpreterGroup.getId(), this.eventClient); | ||
| // throw new TException(e); | ||
| } finally { | ||
| interpreterGroup.setResourcePool(resourcePool); | ||
| return resourcePool; | ||
| } | ||
| } | ||
|
|
||
| @Override | ||
| public void createInterpreter(String interpreterGroupId, String noteId, String | ||
| className, | ||
| Map<String, String> properties) throws TException { | ||
| Map<String, String> properties) throws TException { | ||
| if (interpreterGroup == null) { | ||
| interpreterGroup = new InterpreterGroup(interpreterGroupId); | ||
| angularObjectRegistry = new AngularObjectRegistry(interpreterGroup.getId(), this); | ||
| resourcePool = new DistributedResourcePool(interpreterGroup.getId(), eventClient); | ||
| interpreterGroup.setAngularObjectRegistry(angularObjectRegistry); | ||
| interpreterGroup.setResourcePool(resourcePool); | ||
| } | ||
|
|
||
| try { | ||
|
|
@@ -171,15 +202,19 @@ public void createInterpreter(String interpreterGroupId, String noteId, String | |
| } | ||
|
|
||
| logger.info("Instantiate interpreter {}", className); | ||
|
|
||
| interpreterGroup.setResourcePool(getResourcePool()); | ||
|
|
||
| repl.setInterpreterGroup(interpreterGroup); | ||
|
|
||
| //setResourcePool(interpreterGroup, p, eventClient); | ||
| } catch (ClassNotFoundException | NoSuchMethodException | SecurityException | ||
| | InstantiationException | IllegalAccessException | ||
| | IllegalArgumentException | InvocationTargetException e) { | ||
| logger.error(e.toString(), e); | ||
| throw new TException(e); | ||
| } | ||
| } | ||
|
|
||
| private void setSystemProperty(Properties properties) { | ||
| for (Object key : properties.keySet()) { | ||
| if (!RemoteInterpreter.isEnvString((String) key)) { | ||
|
|
@@ -367,11 +402,13 @@ protected Object jobRun() throws Throwable { | |
| } | ||
|
|
||
| // put result into resource pool | ||
| context.getResourcePool().put( | ||
| context.getNoteId(), | ||
| context.getParagraphId(), | ||
| WellKnownResourceName.ParagraphResult.toString(), | ||
| combinedResult); | ||
| if (context.getResourcePool() != null) { | ||
| context.getResourcePool().put( | ||
| context.getNoteId(), | ||
| context.getParagraphId(), | ||
| WellKnownResourceName.ParagraphResult.toString(), | ||
| combinedResult); | ||
| } | ||
| return combinedResult; | ||
| } finally { | ||
| InterpreterContext.remove(); | ||
|
|
@@ -402,7 +439,7 @@ public void cancel(String noteId, String className, RemoteInterpreterContext int | |
|
|
||
| @Override | ||
| public int getProgress(String noteId, String className, | ||
| RemoteInterpreterContext interpreterContext) | ||
| RemoteInterpreterContext interpreterContext) | ||
| throws TException { | ||
| Interpreter intp = getInterpreter(noteId, className); | ||
| return intp.getProgress(convert(interpreterContext)); | ||
|
|
@@ -425,7 +462,7 @@ public List<String> completion(String noteId, String className, String buf, int | |
| private InterpreterContext convert(RemoteInterpreterContext ric) { | ||
| List<InterpreterContextRunner> contextRunners = new LinkedList<InterpreterContextRunner>(); | ||
| List<InterpreterContextRunner> runners = gson.fromJson(ric.getRunners(), | ||
| new TypeToken<List<RemoteInterpreterContextRunner>>() { | ||
| new TypeToken<List<RemoteInterpreterContextRunner>>() { | ||
| }.getType()); | ||
|
|
||
| for (InterpreterContextRunner r : runners) { | ||
|
|
@@ -586,7 +623,7 @@ public void angularObjectUpdate(String name, String noteId, String paragraphId, | |
| if (value == null) { | ||
| try { | ||
| value = gson.fromJson(object, | ||
| new TypeToken<Map<String, Object>>() { | ||
| new TypeToken<Map<String, Object>>() { | ||
| }.getType()); | ||
| } catch (Exception e) { | ||
| // it's not a generic json object, too. okay, proceed to threat as a string type | ||
|
|
@@ -622,7 +659,7 @@ public void angularObjectAdd(String name, String noteId, String paragraphId, Str | |
| try { | ||
| value = gson.fromJson(object, | ||
| new TypeToken<Map<String, Object>>() { | ||
| }.getType()); | ||
| }.getType()); | ||
| } catch (Exception e) { | ||
| // it's okay. proceed to treat object as a string | ||
| logger.debug(e.getMessage(), e); | ||
|
|
@@ -638,7 +675,7 @@ public void angularObjectAdd(String name, String noteId, String paragraphId, Str | |
|
|
||
| @Override | ||
| public void angularObjectRemove(String name, String noteId, String paragraphId) throws | ||
| TException { | ||
| TException { | ||
| AngularObjectRegistry registry = interpreterGroup.getAngularObjectRegistry(); | ||
| registry.remove(name, noteId, paragraphId, false); | ||
| } | ||
|
|
||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
65 changes: 65 additions & 0 deletions
65
...main/java/org/apache/zeppelin/resource/RemoteInterpreterProcessResourcePoolConnector.java
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,65 @@ | ||
| /* | ||
| * Licensed to the Apache Software Foundation (ASF) under one or more | ||
| * contributor license agreements. See the NOTICE file distributed with | ||
| * this work for additional information regarding copyright ownership. | ||
| * The ASF licenses this file to You under the Apache License, Version 2.0 | ||
| * (the "License"); you may not use this file except in compliance with | ||
| * the License. You may obtain a copy of the License at | ||
| * | ||
| * http://www.apache.org/licenses/LICENSE-2.0 | ||
| * | ||
| * Unless required by applicable law or agreed to in writing, software | ||
| * distributed under the License is distributed on an "AS IS" BASIS, | ||
| * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. | ||
| * See the License for the specific language governing permissions and | ||
| * limitations under the License. | ||
| */ | ||
| package org.apache.zeppelin.resource; | ||
|
|
||
| import java.util.List; | ||
|
|
||
| import org.apache.thrift.TException; | ||
| import org.apache.zeppelin.interpreter.thrift.RemoteInterpreterService.Client; | ||
|
|
||
| import com.google.gson.Gson; | ||
| /** | ||
| * Makes a remote interpreter service client act as a resource pool connector. | ||
| */ | ||
| public class RemoteInterpreterProcessResourcePoolConnector implements ResourcePoolConnector { | ||
|
|
||
| private Client client; | ||
|
|
||
| public RemoteInterpreterProcessResourcePoolConnector(Client client) { | ||
| this.client = client; | ||
| } | ||
|
|
||
| @Override | ||
| public ResourceSet getAllResources() { | ||
| try { | ||
| List<String> resourceList = client.resourcePoolGetAll(); | ||
| ResourceSet resources = new ResourceSet(); | ||
| Gson gson = new Gson(); | ||
|
|
||
| for (String res : resourceList) { | ||
| RemoteResource r = gson.fromJson(res, RemoteResource.class); | ||
| r.setResourcePoolConnector(this); | ||
| resources.add(r); | ||
| } | ||
|
|
||
| return resources; | ||
| } catch (TException e) { | ||
| throw new RuntimeException(e); | ||
| } | ||
| } | ||
|
|
||
| @Override | ||
| public Object readResource(ResourceId id) { | ||
| try { | ||
| // TODO(Object): Deserialize object | ||
| return client.resourceGet(id.getNoteId(), id.getParagraphId(), id.getName()); | ||
| } catch (TException e) { | ||
| throw new RuntimeException(e); | ||
| } | ||
| } | ||
|
|
||
| } |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
68 changes: 68 additions & 0 deletions
68
zeppelin-interpreter/src/main/java/org/apache/zeppelin/resource/ResourceSerializer.java
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,68 @@ | ||
| /* | ||
| * Licensed to the Apache Software Foundation (ASF) under one or more | ||
| * contributor license agreements. See the NOTICE file distributed with | ||
| * this work for additional information regarding copyright ownership. | ||
| * The ASF licenses this file to You under the Apache License, Version 2.0 | ||
| * (the "License"); you may not use this file except in compliance with | ||
| * the License. You may obtain a copy of the License at | ||
| * | ||
| * http://www.apache.org/licenses/LICENSE-2.0 | ||
| * | ||
| * Unless required by applicable law or agreed to in writing, software | ||
| * distributed under the License is distributed on an "AS IS" BASIS, | ||
| * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. | ||
| * See the License for the specific language governing permissions and | ||
| * limitations under the License. | ||
| */ | ||
| package org.apache.zeppelin.resource; | ||
|
|
||
| import java.lang.reflect.Type; | ||
|
|
||
| import com.google.gson.Gson; | ||
| import com.google.gson.JsonDeserializationContext; | ||
| import com.google.gson.JsonDeserializer; | ||
| import com.google.gson.JsonElement; | ||
| import com.google.gson.JsonObject; | ||
| import com.google.gson.JsonParseException; | ||
| import com.google.gson.JsonSerializationContext; | ||
| import com.google.gson.JsonSerializer; | ||
| /** | ||
| * Serializes and Deserializes resources if they are serializable. | ||
| */ | ||
| public class ResourceSerializer implements JsonDeserializer<Resource>, JsonSerializer<Resource> { | ||
|
|
||
| public ResourceSerializer() { | ||
| } | ||
|
|
||
| @Override | ||
| public JsonElement serialize(Resource src, Type typeOfSrc, JsonSerializationContext context) { | ||
| // This is straightforward at the moment. | ||
| Gson gson = new Gson(); | ||
| JsonElement elem = gson.toJsonTree(src); | ||
| JsonObject obj = elem.getAsJsonObject(); | ||
| if (src.isSerializable()) { | ||
| obj.add("r", gson.toJsonTree(src.get())); | ||
| } | ||
| return obj; | ||
| } | ||
|
|
||
| @Override | ||
| public Resource deserialize(JsonElement json, Type typeOfT, JsonDeserializationContext context) | ||
| throws JsonParseException { | ||
| // This requires that we use the class that's stored in the element to deserialize. | ||
| JsonObject obj = json.getAsJsonObject(); | ||
| String className = obj.getAsJsonPrimitive("className").getAsString(); | ||
|
|
||
| Gson gson = new Gson(); | ||
| Object r; | ||
| try { | ||
| r = gson.fromJson(obj.get("r"), Class.forName(className)); | ||
| } catch (ClassNotFoundException e) { | ||
| throw new RuntimeException("Unable to deserialize the resource"); | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. pass e to create the RuntimeException? |
||
| } | ||
| ResourceId id = gson.fromJson(obj.get("resourceId"), ResourceId.class); | ||
|
|
||
| return new Resource(id, r); | ||
| } | ||
|
|
||
| } | ||
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I know we don't have style check for this official but could we put single line into bracket too? It would help readability and prevent possible merge mistakes.
(same for L143)
http://embeddedgurus.com/barr-code/2014/03/apples-gotofail-ssl-security-bug-was-easily-preventable/