openwhisk-runtime-java icon indicating copy to clipboard operation
openwhisk-runtime-java copied to clipboard

Add OpenJDK 11 action loop runtime

Open upgle opened this issue 5 years ago • 18 comments

This PR is to support OpenJDK 11. https://github.com/apache/incubator-openwhisk-runtime-java/issues/78

upgle avatar Apr 09 '19 09:04 upgle

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/

mrutkows avatar Jul 29 '19 22:07 mrutkows

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."

mrutkows avatar Jul 29 '19 22:07 mrutkows

The conflict has been resolved

upgle avatar Sep 24 '19 06:09 upgle

@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 avatar Nov 06 '19 16:11 mrutkows

@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?

  • case 1) Add actionLoop based Java11 (actionloop-java-v11), legacy Java 11 (java11action)
  • case 2) Add only actionLoop based Java11 (actionloop-java-v11)

If we don't need to care lagacy code anymore, I'll convert this runtime as actionLoop based runtime as well.

upgle avatar Feb 25 '20 08:02 upgle

+1 for one version - 2 versions means twice the maintenance.

rabbah avatar Feb 25 '20 12:02 rabbah

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

upgle avatar Feb 26 '20 04:02 upgle

I've converted the legacy java11 runtime to use action loop.

upgle avatar Feb 26 '20 08:02 upgle

@mrutkows Do you have any other comments on this?

style95 avatar Mar 23 '20 10:03 style95

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.

rabbah avatar Mar 23 '20 16:03 rabbah

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...

sciabarra avatar Mar 28 '20 11:03 sciabarra

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.

sciabarra avatar Apr 04 '20 08:04 sciabarra

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:

image

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.

mattwelke avatar Jun 01 '21 04:06 mattwelke

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.

rabbah avatar Jun 03 '21 21:06 rabbah

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.

mattwelke avatar Jun 03 '21 21:06 mattwelke

The context parameter is a pattern that will work all runtimes.

Are you prepared to push this change forward for all the runtimes ;)

rabbah avatar Jun 03 '21 22:06 rabbah

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:

  • Node.js
  • Go
  • Java
  • C#/dotnet
  • PHP
  • Python
  • Ruby

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.

mattwelke avatar Jun 03 '21 23:06 mattwelke

Thanks @mattwelke for leading this discussion. There will def be help once the proposal matures.

rabbah avatar Jun 04 '21 13:06 rabbah