search

amino-os / Amino.Run

Amino Distributed OS - Runtime Manager
Apache License 2.0
29 stars 12 forks source link

[Bug] Fix Ruby client failure post Graal RC14 upgrade #725

Closed AmitRoushan closed 5 years ago

AmitRoushan commented 5 years ago

Post Graal RC14 upgrade Ruby client with Ruby micro service support need fix. Following is issue faced:

  /home/roo1/development/DCAP-Sapphire/examples/hanksTodoRuby/src/main/ruby/amino/run/appexamples/hankstodo/hanks_todo_main.rb:22:in `main': The value 'DynamicObject@115c946b<Array>' cannot be passed from one context to another. The current context is 0x79ca7bea and the argument value originates from context 0x54f6b629. (ArgumentError)
     from /home/roo1/development/DCAP-Sapphire/examples/hanksTodoRuby/src/main/ruby/amino/run/appexamples/hankstodo/hanks_todo_main.rb:40:in `<main>'

Umbrella issue #658

AmitRoushan commented 5 years ago

FYI : primitive data types are working fine. Issues are there in composite or user defined data type Similar issue : https://github.com/oracle/graal/issues/631

AmitRoushan commented 5 years ago

As mentioned above ruby client - ruby server scenario works well with primitive type. Root cause for Composite/UserDefine type failure: App stub(TodoListManager_Stub) in multi language uses a org.graalvm.polyglot.Context instance for deserialization. It is able to successfully deserialize it but when it try to return it to Ruby Client, Ruby Client tries to parse it with different context. Till GraalVM RC8 the support was available to pass value from one context to other but after RC9 only primitive, host and proxy objects are allowed.

To validate it, We have tested HanksTodoRuby app with RC6. RC7 and RC8. It works fine till RC8. We also tested HanksTodoRuby app with RC16 and it fails with user defined class.

Change log in RC9: https://github.com/oracle/graal/blob/master/sdk/CHANGELOG.md#version-10-rc9

prostil commented 5 years ago

This issue can be closed as it is being tracked by another issue #658

quinton-hoole commented 5 years ago

Re-opening, as we can't remain stuck on RC8 forever. RC19 is now out. We need to fix this. Hopefully we can use a single Context, and the cross-context problem will go away.

maheshrajus commented 5 years ago

@quinton-hoole : 1) I tested with GraalVM RC19 version and issue(Illigeal argument Exception) still exist with composite type arguments. 2) Tested KvStoreJS example with RC19 GraalVm.

image

image

3) As per your comment related to single conext (https://github.com/amino-os/Amino.Run/issues/725#issuecomment-497399281), I am checking workaround. Please give me any suggestions on the same. Thank you!

quinton-hoole commented 5 years ago

I have not looked into it at all. I just understood that we have two Graal Contexts and are trying to access between them, which is not allowed. Is it possible to place the objects in the same Graal Context?

maheshrajus commented 5 years ago

@quinton-hoole : I am not sure about placing objects in the single Graal Context. I am checking it currently. I will confirm it after my analysis.

maheshrajus commented 5 years ago

@quinton-hoole : As per discussion with the graalvm community we can not serialize and use the same context at the client side. They have given some other way to achieve this. We have to convert it to JSON string and send to client side and use it. I am doing this workaround currently.

Discussion on Gitter: https://gitter.im/graalvm/graal-core : Christian Humer @chumer 20:06 @maheshrajus No we don’t currently support serialization of polyglot contexts or values. You may convert objects to a string representation like json to send objects back and forth between server/client. Mahesh Raju Somalaraju @maheshrajus Jun 07 20:12 ok, thank you @chumer

quinton-hoole commented 5 years ago

@maheshrajus Please avoid using JSON as the performance is (highly likely to be) very poor. Please use swift or protobuf instead (or at the very least measure the performance difference between these three before proceeding).

quinton-hoole commented 5 years ago

Also, why is this only a problem for one particular language?

maheshrajus commented 5 years ago

@maheshrajus Please avoid using JSON as the performance is (highly likely to be) very poor. Please use swift or protobuf instead (or at the very least measure the performance difference between these three before proceeding).

@quinton-hoole : While converting to JSON string,I received some exceptions and checked the same with Graal community people. context object is not data object we can not convert to json. We have to check some other workaround other than single context.

image image image

maheshrajus commented 5 years ago

Also, why is this only a problem for one particular language?

Yeah, I checked with java client with JS server and it is working perfectly. In this case also we are passing the non-primitive data object to context obj which created at the client side for deserializing the data. I am continuing my further analysis.

image image

quinton-hoole commented 5 years ago

To clarify, #817 does not fix this issue.

The automated example scripts were simply not running the problematic case, and hence no errors were reported.

The reported error is still as before, when run correctly:

$ ./gradlew :examples:hanksTodoRuby:runjsapp

> Task :examples:hanksTodoRuby:runjsapp
Warning: '--jvm.*' options are deprecated, use '--vm.*' instead.
constructor
constructor
OK_add_todo
OK_add_todo
OK_add_todo
java.lang.IllegalArgumentException: The value 'DynamicObject<JSArray>@2796aeae' cannot be passed from one context to another. The current context is 0xb4711e2 and the argument value originates from context 0x1fa1cab1.
        at com.oracle.truffle.polyglot.PolyglotLanguageContext.migrateValue(PolyglotLanguageContext.java:861)
        at com.oracle.truffle.polyglot.PolyglotLanguageContext.migrateHostWrapper(PolyglotLanguageContext.java:872)

I will now get to work on fixing the above, as well as making sure that the automated tests run all the required scenarios (i.e. all client languages, against all server languages, and with non-primitive types).

quinton-hoole commented 5 years ago

@maheshrajus Did you get any further with your analysis of this issue?

quinton-hoole commented 5 years ago

@maheshrajus By the way, great work on this so far!

quinton-hoole commented 5 years ago

@maheshrajus Unfortunately it seems that there was a bug in the test you used to verify that java client to js server was working correctly. It seems that two unrelated instances of Date are being created and initialized to the current time. It just happened that the current time was within the same second, and the resolution of the printed output therefore made them look identical. I fixed the test as per below, and the values of the Date are indeed different: Code: 

            if (i % 2 == 0) val = new Date();
            else val = new TestClass(i);

            String valStr = val instanceof Date ? String.valueOf(((Date)val).getTime()) : String.valueOf(val);
            System.out.println(String.format("<client>: setting %s = %s", key, valStr));
            store.set(key, val);
            val = store.get(key);
            valStr = val instanceof Date ? String.valueOf(((Date)val).getTime()) : String.valueOf(val);
            System.out.println(String.format("<client>: got value %s with key %s", valStr, key));

Output:

<client>: setting key_2 = 1568825290723
Sep 18, 2019 9:48:10 AM amino.run.policy.scalability.LoadBalancedMasterSlaveBase$ClientPolicy onRPC
INFO: Sending request to master server localhost/127.0.0.1:22345
Sep 18, 2019 9:48:10 AM amino.run.policy.scalability.LoadBalancedMasterSlaveBase$ClientPolicy onRPC
INFO: Sending request to master server localhost/127.0.0.1:22345
<client>: got value 1568825290818 with key key_2