Open upgle opened 5 years ago
For canonical discussion on illegal access: http://mail.openjdk.java.net/pipermail/jigsaw-dev/2017-May/012673.html
See blog discussion of migrating to Java 11, addressing the issue as well: https://blog.codefx.org/java/java-11-migration-guide/
NOTE:
--illegal-access=permit
"This will be the default mode for JDK 9. It opens every package in every explicit module to code in all unnamed modules, i.e., code on the class path, just as --permit-illegal-access
does today."
The conflict has been resolved
@upgle First, I wanted to apologize for not seeing your resolving conflicts on Set 24th. I have been busy and traveling and am now back and have time to dedicate to openwhisk and the java runtime. I hate to ask this, but could you resolve the gradle config conflicts and I promise to review/merge as soon as you complete. Feel free to message me directly in Slack if you like to assure I know the minute you are done or just to chat and ask questions. Many thanks!
@mrutkows I'm sorry for the late response too, I'll resolve all conflicts soon.
When I made this Java11 runtime, I didn't consider the actionLoop. Which do you think is better, making both versions or just one version?
If we don't need to care lagacy code anymore, I'll convert this runtime as actionLoop based runtime as well.
+1 for one version - 2 versions means twice the maintenance.
Ok, I'll convert this code for action-loop based.
And I have a question @rabbah.
Currently, the python runtime has both actionLoop-based and non-actionLoop-based runtimes. Do you plan to support both types even if the python version is updated? I just wonder if actionLoop has become the standard specification for the runtime.
https://github.com/apache/openwhisk-runtime-python/tree/master/core
I've converted the legacy java11 runtime to use action loop.
@mrutkows Do you have any other comments on this?
If the action loop proxy sets all the environment variables for the activation, then I posit the unsafe env setting in the jvm isn’t needed anymore and so the unsafe hack could be removed. I’d favor making sure the env vars are all set in the proxy before the jvm starts vs switching to properties as that’s a breaking change and will break action migration from java8 to java10 kinds.
The actionloop cannot set the environment variables for the activation, it can set the environment variables BEFORE the process start but it cannot change environment variables of a launched process. No program can do this, there is in Unix an exec call where you can pass an environment variables array. It is the task of a the launched program to set environment variables. Indeed all the "action loops" does exactly this: read the action and sets the environment variables. This is easy in every language except java, where it has been decided you cannot do this. And the only way is the ugly hack of marking an in-memory variabile read-only as writable and write it. I proposed to remove this hack and instead accept that the activation values are passed as system properties. This requires to change and document the difference. Or use the unsafe code...
My opinion is that we should NOT use --add-opens and --illegal-access, but instead convert the way values are passed as system properties, and document the change. I discussed this in mailing list.
I agree that it's a bad idea to use flags like --add-opens and -illegal-access. I think the best solution would be to change the contract between the runtime and the user action code so that those hacks aren't needed.
I think the "OpenWhisk variables" as I like to call them (the env vars that start with __OW_
) just need to get to the user action code in any way, as long as it's secure. The OpenWhisk user (making an action) probably doesn't care how. I thought it would make sense for them to just be included in the parameters object the user action is already expected to have. To make things safer, in case users have code like "log all parameters" in their actions, it could be a second parameter. That way, the first parameter is the function's parameters (set at deploy time, with overrides per request) and the second parameter is these more internal OpenWhisk variables like API key.
I made a Java 11 runtime to test this out: https://github.com/mattwelke/apache-openwhisk-runtime-java
It works pretty well, as you can see from the screenshot in that repo's README:
Note that I'm also using a technique I mentioned in https://github.com/apache/openwhisk-runtime-dotnet/issues/51, where I suggest using the language's native map data structure instead of JSON for the user action contract (as the Node.js and Go runtimes already do) so that the user doesn't have to link to a JSON library dependency to build their action code. In Java, that means java.util.Map
.
My custom runtime repo's example action shows it all put together, which is what's running in that IBM Cloud Function:
import java.util.HashMap;
import java.util.Map;
public class Function {
public static Map<String, Object> main(Map<String, Object> params, Map<String, Object> owVars) {
System.out.println("Runtime version: " + Runtime.version());
printMap(params, "Params");
printMap(owVars, "OpenWhisk variables");
var output = new HashMap<String, Object>();
output.put("hello", "world");
output.put("answer", 42);
return output;
}
private static void printMap(Map<String, Object> map, String mapName) {
System.out.println(mapName + " entries:");
for (var entry : map.entrySet()) {
System.out.println("Key: " + entry.getKey() + " | Value: " + entry.getValue());
}
}
}
Note that if that repo's layout looks weird to you, it's not you. :P I'm not using Maven or Gradle because I haven't gotten a chance to know them deeply yet. I'm learning Java because my work is pivoting to using it as our main language. I thought a fun way to learn Java would be to make Java 11 and Java 16 runtimes for OpenWhisk. But it's easier for me to put together a custom build system using Bash and checked in JAR files than to figure out Maven or Gradle right now. I understand that if we add a Java 11 runtime to the project, we'd want to make its structure fit our existing Gradle build system.
The second set of parameters is called context in AWS Lambda lingo. Did you try system properties instead of environment variables? As I noted on the dev list, I think once we change the function signature for one language, it opens up a broader question of whether we should do that for all languages. The advantage of the OW programming model is that it's uniform across languages today - the signature is dictionary in, dictionary out. Lambda has a bunch of different signatures over time for different languages. I think they're getting better over time.
Did you try system properties instead of environment variables? As I noted on the dev list, I think once we change the function signature for one language, it opens up a broader question of whether we should do that for all languages. The advantage of the OW programming model is that it's uniform across languages today
This is actually why I leaned towards a context parameter instead of system properties for Java. The context parameter is a pattern that will work all runtimes. It would let OpenWhisk continue to have that consistency. System properties are a Java only concept, so this would only work for Java and if another runtime ever has this come up too, then we'd have to use a language-specific fix for that runtime too.
Alternatively, we could say that consistency is only a goal within a particular language's runtimes. Maybe as long as all Java runtimes use system properties, we're good. And then it's okay to use other techniques (like JsonObject for C#, and language native dictionaries for Node.js and Go). It depends on what kind of consistency design goals we have.
Of these two types of consistency, my gut feeling is that consistency across languages is better, using language native dictionaries and the context parameter. When an OpenWhisk user makes the jump from one language to another, like me when I moved from Node.js to C# when I wanted a compiler, and then C# to Java when my work began using Java and I wanted to skill up on it, then the user has less context shifting to worry about.
The context parameter is a pattern that will work all runtimes.
Are you prepared to push this change forward for all the runtimes ;)
Are you prepared to push this change forward for all the runtimes ;)
While I wouldn't say I'm skilled enough in all of these languages to have made the runtimes from scratch, I think this is actually a simple change to make. I found it easy to make in Java. I'd say that my strongest languages are Node.js and Go right now, and that I'd feel comfortable pushing this change forward in the following runtimes, all of which I've coded at least a bit throughout my life as a programmer:
The other runtimes, I haven't ever ran a single line of code in them (like Swift) so there'd probably be some friction there, getting set up and maybe running into little things that would be obvious to someone with experience with the languages. I'd appreciate some help from others to step in and do those ones.
As we discussed on Slack, there was a reason it was done with environment variables in the first place, to enable composition. And we'd want to explore whether composition would be hurt by changing it this way first.
Thanks @mattwelke for leading this discussion. There will def be help once the proposal matures.
This PR is to support OpenJDK 11. https://github.com/apache/incubator-openwhisk-runtime-java/issues/78