Merge lp:~mmcm/akiban-server/script-invoker-pool into lp:~akiban-technologies/akiban-server/trunk

Proposed by Mike McMahon
Status: Merged
Approved by: Nathan Williams
Approved revision: 2620
Merged at revision: 2620
Proposed branch: lp:~mmcm/akiban-server/script-invoker-pool
Merge into: lp:~akiban-technologies/akiban-server/trunk
Diff against target: 460 lines (+171/-79)
8 files modified
src/main/java/com/akiban/server/service/restdml/DirectServiceImpl.java (+23/-6)
src/main/java/com/akiban/server/service/routines/RoutineLoader.java (+1/-0)
src/main/java/com/akiban/server/service/routines/RoutineLoaderImpl.java (+5/-0)
src/main/java/com/akiban/server/service/routines/ScriptCache.java (+107/-68)
src/main/java/com/akiban/server/service/routines/ScriptInvoker.java (+1/-3)
src/main/java/com/akiban/server/service/routines/ScriptLibrary.java (+25/-0)
src/main/java/com/akiban/sql/script/ScriptFunctionJavaRoutine.java (+4/-2)
src/test/java/com/akiban/server/service/routines/MockRoutineLoader.java (+5/-0)
To merge this branch: bzr merge lp:~mmcm/akiban-server/script-invoker-pool
Reviewer Review Type Date Requested Status
Nathan Williams Approve
Review via email: mp+158507@code.launchpad.net

Description of the change

Clean up ScriptInvoker as mentioned in lp:~pbeaman/akiban-server/direct-param-handling/+merge/156711/comments/343411

Now distinguishes the invoker proper from the library from which it comes. There are still no cases where more than one routine comes from the same library (a functional calling convention has one and a direct module has zero), but the pool itself now supports it, pooling the big ScriptEngine/Invocable once for all.

To post a comment you must log in.
Revision history for this message
Nathan Williams (nwilliams) wrote :

Looks plausible.

review: Approve

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'src/main/java/com/akiban/server/service/restdml/DirectServiceImpl.java'
2--- src/main/java/com/akiban/server/service/restdml/DirectServiceImpl.java 2013-04-11 00:14:28 +0000
3+++ src/main/java/com/akiban/server/service/restdml/DirectServiceImpl.java 2013-04-11 23:35:24 +0000
4@@ -62,7 +62,8 @@
5 import com.akiban.server.service.restdml.EndpointMetadata.ParamMetadata;
6 import com.akiban.server.service.restdml.EndpointMetadata.Tokenizer;
7 import com.akiban.server.service.routines.RoutineLoader;
8-import com.akiban.server.service.routines.ScriptInvoker;
9+import com.akiban.server.service.routines.ScriptLibrary;
10+import com.akiban.server.service.routines.ScriptPool;
11 import com.akiban.server.service.security.SecurityService;
12 import com.akiban.server.service.session.Session;
13 import com.akiban.server.types3.Attribute;
14@@ -394,9 +395,19 @@
15
16 final Object[] args = createArgsArray(pathParams, queryParameters, content, cache, md);
17
18- final ScriptInvoker invoker = conn.getRoutineLoader()
19- .getScriptInvoker(conn.getSession(), new TableName(procName.getSchemaName(), md.routineName)).get();
20- Object result = invoker.invokeNamedFunction(md.function, args);
21+ final ScriptPool<ScriptLibrary> libraryPool = conn.getRoutineLoader()
22+ .getScriptLibrary(conn.getSession(), new TableName(procName.getSchemaName(),
23+ md.routineName));
24+ final ScriptLibrary library = libraryPool.get();
25+ boolean success = false;
26+ Object result;
27+ try {
28+ result = library.invoke(md.function, args);
29+ success = true;
30+ }
31+ finally {
32+ libraryPool.put(library, success);
33+ }
34
35 switch (md.outParam.type) {
36
37@@ -529,9 +540,11 @@
38 for (final Routine routine : ais.getRoutines().values()) {
39 if (routine.getCallingConvention().equals(CallingConvention.SCRIPT_LIBRARY)
40 && routine.getDynamicResultSets() == 0 && routine.getParameters().isEmpty()) {
41+ final ScriptPool<ScriptLibrary> libraryPool = routineLoader.getScriptLibrary(session, routine.getName());
42+ final ScriptLibrary library = libraryPool.get();
43+ boolean success = false;
44 try {
45- final ScriptInvoker invoker = routineLoader.getScriptInvoker(session, routine.getName()).get();
46- invoker.invokeNamedFunction(DISTINGUISHED_REGISTRATION_METHOD_NAME,
47+ library.invoke(DISTINGUISHED_REGISTRATION_METHOD_NAME,
48 new Object[] { new RestFunctionRegistrar() {
49 @Override
50 public void register(String specification) throws Exception {
51@@ -539,9 +552,11 @@
52 .getTableName(), specification);
53 }
54 } });
55+ success = true;
56 } catch (ExternalRoutineInvocationException e) {
57 if (e.getCause() instanceof NoSuchMethodException) {
58 LOG.warn("Script library " + routine.getName() + " has no _register function");
59+ success = true;
60 return;
61 }
62 Throwable previous = e;
63@@ -557,6 +572,8 @@
64 throw e;
65 } catch (Exception e) {
66 throw new RegistrationException(e);
67+ } finally {
68+ libraryPool.put(library, success);
69 }
70 }
71 }
72
73=== modified file 'src/main/java/com/akiban/server/service/routines/RoutineLoader.java'
74--- src/main/java/com/akiban/server/service/routines/RoutineLoader.java 2013-03-22 20:05:57 +0000
75+++ src/main/java/com/akiban/server/service/routines/RoutineLoader.java 2013-04-11 23:35:24 +0000
76@@ -33,5 +33,6 @@
77 public boolean isScriptLanguage(Session session, String language);
78 public ScriptPool<ScriptEvaluator> getScriptEvaluator(Session session, TableName routineName);
79 public ScriptPool<ScriptInvoker> getScriptInvoker(Session session, TableName routineName);
80+ public ScriptPool<ScriptLibrary> getScriptLibrary(Session session, TableName routineName);
81 public void unloadRoutine(Session session, TableName routineName);
82 }
83
84=== modified file 'src/main/java/com/akiban/server/service/routines/RoutineLoaderImpl.java'
85--- src/main/java/com/akiban/server/service/routines/RoutineLoaderImpl.java 2013-03-22 20:05:57 +0000
86+++ src/main/java/com/akiban/server/service/routines/RoutineLoaderImpl.java 2013-04-11 23:35:24 +0000
87@@ -194,6 +194,11 @@
88 }
89
90 @Override
91+ public ScriptPool<ScriptLibrary> getScriptLibrary(Session session, TableName routineName) {
92+ return scripts.getScriptLibrary(session, routineName);
93+ }
94+
95+ @Override
96 public void unloadRoutine(Session session, TableName routineName) {
97 synchronized (loadablePlans) {
98 loadablePlans.remove(routineName);
99
100=== modified file 'src/main/java/com/akiban/server/service/routines/ScriptCache.java'
101--- src/main/java/com/akiban/server/service/routines/ScriptCache.java 2013-04-03 13:41:34 +0000
102+++ src/main/java/com/akiban/server/service/routines/ScriptCache.java 2013-04-11 23:35:24 +0000
103@@ -61,8 +61,12 @@
104 return getEntry(session, routineName).getScriptEvaluator();
105 }
106
107+ public ScriptPool<ScriptLibrary> getScriptLibrary(Session session, TableName routineName) {
108+ return getEntry(session, routineName).getScriptLibrary();
109+ }
110+
111 public ScriptPool<ScriptInvoker> getScriptInvoker(Session session, TableName routineName) {
112- return getEntry(session, routineName).getScriptInvoker();
113+ return getEntry(session, routineName).getScriptInvoker(this, session);
114 }
115
116 protected ScriptEngineManager getManager(Session session) {
117@@ -85,23 +89,25 @@
118 return entry;
119 }
120
121- static class CacheEntry {
122+ class CacheEntry {
123 private TableName routineName;
124 private String script;
125+ private TableName libraryName;
126 private String function;
127 private ScriptEngineFactory factory;
128 private String threading;
129 private boolean invocable, compilable;
130 private ScriptPool<ScriptEvaluator> sharedEvaluatorPool;
131- private ScriptPool<ScriptInvoker> sharedInvokerPool;
132+ private ScriptPool<ScriptLibrary> sharedLibraryPool;
133 private ScriptEngine spareEngine;
134
135 public CacheEntry(Routine routine, ScriptEngine engine) {
136 routineName = routine.getName();
137 script = routine.getDefinition();
138+ libraryName = routineName; // TODO: Until qualified EXTERNAL NAME supported.
139 function = routine.getMethodName();
140 factory = engine.getFactory();
141- threading = (String) factory.getParameter("THREADING");
142+ threading = (String)factory.getParameter("THREADING");
143 invocable = (engine instanceof Invocable);
144 compilable = (engine instanceof Compilable);
145 spareEngine = engine;
146@@ -127,9 +133,8 @@
147 }
148 }
149
150- // Otherwise, every caller gets a new pool which only has the scope
151- // of the
152- // prepared statement, etc.
153+ // Otherwise, every caller gets a new pool which only has
154+ // the scope of the prepared statement, etc.
155 ScriptEngine engine;
156 synchronized (this) {
157 engine = spareEngine;
158@@ -149,42 +154,54 @@
159 }
160 }
161
162- public ScriptPool<ScriptInvoker> getScriptInvoker() {
163- // TODO - talk with Mike about having a CacheEntry without a function -
164- // needed for the LIBRARY param styl
165- assert invocable ; //&& (function != null);
166- // Can share if at multi-threaded (or stronger), since we are
167- // invoking
168- // the function.
169- if ("MULTITHREADED".equals(threading) || "THREAD-ISOLATED".equals(threading)
170- || "STATELESS".equals(threading)) {
171+ public ScriptPool<ScriptLibrary> getScriptLibrary() {
172+ assert invocable;
173+ // Can share if at multi-threaded (or stronger), since we
174+ // are invoking the function.
175+ if ("MULTITHREADED".equals(threading) ||
176+ "THREAD-ISOLATED".equals(threading) ||
177+ "STATELESS".equals(threading)) {
178 synchronized (this) {
179- if (sharedInvokerPool == null) {
180+ if (sharedLibraryPool == null) {
181 ScriptEngine engine = spareEngine;
182 if (engine != null)
183 spareEngine = null;
184 else
185 engine = factory.getScriptEngine();
186- ScriptInvoker invoker = new Invoker(routineName, engine, script, function);
187- sharedInvokerPool = new SharedPool<>(invoker);
188+ ScriptLibrary library = new Library(routineName, engine, script);
189+ sharedLibraryPool = new SharedPool<>(library);
190 }
191- return sharedInvokerPool;
192+ return sharedLibraryPool;
193 }
194 }
195
196- // Otherwise, every caller gets a new pool which only has the scope
197- // of the
198- // prepared statement, etc.
199+ // Otherwise, every caller gets a new pool which only has
200+ // the scope of the prepared statement, etc.
201 ScriptEngine engine;
202 synchronized (this) {
203 engine = spareEngine;
204 if (engine != null)
205 spareEngine = null;
206 }
207- Invoker invoker = null;
208+ Library library = null;
209 if (engine != null)
210- invoker = new Invoker(routineName, engine, script, function);
211- return new InvokerPool(routineName, factory, script, function, invoker);
212+ library = new Library(routineName, engine, script);
213+ return new LibraryPool(routineName, factory, script, library);
214+ }
215+
216+ public ScriptPool<ScriptInvoker> getScriptInvoker(ScriptCache cache, Session session) {
217+ assert invocable && (function != null);
218+ ScriptPool<ScriptLibrary> libraryPool;
219+ if (routineName.equals(libraryName)) {
220+ libraryPool = getScriptLibrary();
221+ }
222+ else {
223+ synchronized (this) {
224+ spareEngine = null;
225+ }
226+ libraryPool = cache.getScriptLibrary(session, libraryName);
227+ }
228+ return new InvokerPool(libraryPool, function);
229 }
230 }
231
232@@ -277,18 +294,14 @@
233 }
234 }
235
236- static class InvokerPool extends BasePool<ScriptInvoker> {
237- private final String function;
238-
239- public InvokerPool(TableName routineName, ScriptEngineFactory factory, String script, String function,
240- Invoker initial) {
241+ static class LibraryPool extends BasePool<ScriptLibrary> {
242+ public LibraryPool(TableName routineName, ScriptEngineFactory factory, String script, Library initial) {
243 super(routineName, factory, script, initial);
244- this.function = function;
245 }
246
247 @Override
248- protected Invoker create() {
249- return new Invoker(routineName, factory.getScriptEngine(), script, function);
250+ protected Library create() {
251+ return new Library(routineName, factory.getScriptEngine(), script);
252 }
253 }
254
255@@ -396,14 +409,12 @@
256 }
257 }
258
259- static class Invoker implements ScriptInvoker {
260+ static class Library implements ScriptLibrary {
261 private final TableName routineName;
262- private final String function;
263 private final Invocable invocable;
264
265- public Invoker(TableName routineName, ScriptEngine engine, String script, String function) {
266+ public Library(TableName routineName, ScriptEngine engine, String script) {
267 this.routineName = routineName;
268- this.function = function;
269 setScriptName(routineName, engine);
270 try {
271 if (engine instanceof Compilable) {
272@@ -420,42 +431,70 @@
273 }
274
275 @Override
276+ public String getEngineName() {
277+ return ((ScriptEngine) invocable).getFactory().getEngineName();
278+ }
279+
280+ @Override
281+ public boolean isCompiled() {
282+ return (invocable instanceof Compilable);
283+ }
284+
285+ @Override
286+ public Object invoke(String function, Object[] args) {
287+ logger.debug("Calling {} in {}", function, routineName);
288+ try {
289+ return invocable.invokeFunction(function, args);
290+ } catch (ScriptException ex) {
291+ throw new ExternalRoutineInvocationException(routineName, ex);
292+ } catch (NoSuchMethodException ex) {
293+ throw new ExternalRoutineInvocationException(routineName, ex);
294+ }
295+ }
296+ }
297+
298+ static class InvokerPool implements ScriptPool<ScriptInvoker> {
299+ private final ScriptPool<ScriptLibrary> libraryPool;
300+ private final String function;
301+
302+ public InvokerPool(ScriptPool<ScriptLibrary> libraryPool, String function) {
303+ this.libraryPool = libraryPool;
304+ this.function = function;
305+ }
306+
307+ @Override
308+ public ScriptInvoker get() {
309+ return new Invoker(libraryPool.get(), function);
310+ }
311+
312+ @Override
313+ public void put(ScriptInvoker elem, boolean success) {
314+ libraryPool.put(elem.getLibrary(), success);
315+ }
316+ }
317+
318+ static class Invoker implements ScriptInvoker {
319+ private final ScriptLibrary library;
320+ private final String function;
321+
322+ public Invoker(ScriptLibrary library, String function) {
323+ this.library = library;
324+ this.function = function;
325+ }
326+
327+ @Override
328+ public ScriptLibrary getLibrary() {
329+ return library;
330+ }
331+
332+ @Override
333 public String getFunctionName() {
334 return function;
335 }
336
337 @Override
338- public String getEngineName() {
339- return ((ScriptEngine) invocable).getFactory().getEngineName();
340- }
341-
342- @Override
343- public boolean isCompiled() {
344- return (invocable instanceof Compilable);
345- }
346-
347- @Override
348 public Object invoke(Object[] args) {
349- logger.debug("Calling {} in {}", function, routineName);
350- try {
351- return invocable.invokeFunction(function, args);
352- } catch (ScriptException ex) {
353- throw new ExternalRoutineInvocationException(routineName, ex);
354- } catch (NoSuchMethodException ex) {
355- throw new ExternalRoutineInvocationException(routineName, ex);
356- }
357- }
358-
359- @Override
360- public Object invokeNamedFunction(String functionName, Object[] args) {
361- logger.debug("Calling {} in {}", function, routineName);
362- try {
363- return invocable.invokeFunction(functionName, args);
364- } catch (ScriptException ex) {
365- throw new ExternalRoutineInvocationException(routineName, ex);
366- } catch (NoSuchMethodException ex) {
367- throw new ExternalRoutineInvocationException(routineName, ex);
368- }
369+ return library.invoke(function, args);
370 }
371 }
372 }
373
374=== modified file 'src/main/java/com/akiban/server/service/routines/ScriptInvoker.java'
375--- src/main/java/com/akiban/server/service/routines/ScriptInvoker.java 2013-03-28 21:05:55 +0000
376+++ src/main/java/com/akiban/server/service/routines/ScriptInvoker.java 2013-04-11 23:35:24 +0000
377@@ -19,9 +19,7 @@
378
379 public interface ScriptInvoker
380 {
381- public String getEngineName();
382+ public ScriptLibrary getLibrary();
383 public String getFunctionName();
384- public boolean isCompiled();
385 public Object invoke(Object[] args);
386- public Object invokeNamedFunction(String functionName, Object[] args);
387 }
388
389=== added file 'src/main/java/com/akiban/server/service/routines/ScriptLibrary.java'
390--- src/main/java/com/akiban/server/service/routines/ScriptLibrary.java 1970-01-01 00:00:00 +0000
391+++ src/main/java/com/akiban/server/service/routines/ScriptLibrary.java 2013-04-11 23:35:24 +0000
392@@ -0,0 +1,25 @@
393+/**
394+ * Copyright (C) 2009-2013 Akiban Technologies, Inc.
395+ *
396+ * This program is free software: you can redistribute it and/or modify
397+ * it under the terms of the GNU Affero General Public License as published by
398+ * the Free Software Foundation, either version 3 of the License, or
399+ * (at your option) any later version.
400+ *
401+ * This program is distributed in the hope that it will be useful,
402+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
403+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
404+ * GNU Affero General Public License for more details.
405+ *
406+ * You should have received a copy of the GNU Affero General Public License
407+ * along with this program. If not, see <http://www.gnu.org/licenses/>.
408+ */
409+
410+package com.akiban.server.service.routines;
411+
412+public interface ScriptLibrary
413+{
414+ public String getEngineName();
415+ public boolean isCompiled();
416+ public Object invoke(String function, Object[] args);
417+}
418
419=== modified file 'src/main/java/com/akiban/sql/script/ScriptFunctionJavaRoutine.java'
420--- src/main/java/com/akiban/sql/script/ScriptFunctionJavaRoutine.java 2013-03-22 20:05:57 +0000
421+++ src/main/java/com/akiban/sql/script/ScriptFunctionJavaRoutine.java 2013-04-11 23:35:24 +0000
422@@ -32,6 +32,7 @@
423 import com.akiban.server.explain.Label;
424 import com.akiban.server.explain.PrimitiveExplainer;
425 import com.akiban.server.service.routines.ScriptInvoker;
426+import com.akiban.server.service.routines.ScriptLibrary;
427 import com.akiban.server.service.routines.ScriptPool;
428
429 import java.sql.ResultSet;
430@@ -129,11 +130,12 @@
431 public CompoundExplainer getExplainer(ExplainContext context) {
432 Attributes atts = new Attributes();
433 ScriptInvoker invoker = pool.get();
434+ ScriptLibrary library = invoker.getLibrary();
435 atts.put(Label.PROCEDURE_IMPLEMENTATION,
436- PrimitiveExplainer.getInstance(invoker.getEngineName()));
437+ PrimitiveExplainer.getInstance(library.getEngineName()));
438 atts.put(Label.PROCEDURE_IMPLEMENTATION,
439 PrimitiveExplainer.getInstance(invoker.getFunctionName()));
440- if (invoker.isCompiled())
441+ if (library.isCompiled())
442 atts.put(Label.PROCEDURE_IMPLEMENTATION,
443 PrimitiveExplainer.getInstance("compiled"));
444 pool.put(invoker, true);
445
446=== modified file 'src/test/java/com/akiban/server/service/routines/MockRoutineLoader.java'
447--- src/test/java/com/akiban/server/service/routines/MockRoutineLoader.java 2013-03-22 20:05:57 +0000
448+++ src/test/java/com/akiban/server/service/routines/MockRoutineLoader.java 2013-04-11 23:35:24 +0000
449@@ -62,6 +62,11 @@
450 }
451
452 @Override
453+ public ScriptPool<ScriptLibrary> getScriptLibrary(Session session, TableName routineName) {
454+ throw new UnsupportedOperationException();
455+ }
456+
457+ @Override
458 public void unloadRoutine(Session session, TableName routineName) {
459 throw new UnsupportedOperationException();
460 }

Subscribers

People subscribed via source and target branches