quarkiverse / quarkus-amazon-services

Quarkus Amazon Services extensions
Apache License 2.0
39 stars 48 forks source link

@DynamoDBVersionAttribute with dynamodb-enhanced not working in native mode #416

Closed dagrammy closed 1 year ago

dagrammy commented 1 year ago

First of all, thanks for your work on Quarkus/Quarkiverse. We really enjoy working with Quarkus. :)

We are using Quarkus version 2.14.3.Final and quarkus-amazon-services version 1.3.1

To access DynamoDb we use the quarkus-amazon-dynamodb-enhanced extension. Now we want to introduce opmtistic locking for an entity. https://docs.aws.amazon.com/amazondynamodb/latest/developerguide/DynamoDBMapper.OptimisticLocking.html

We added a field with the annotation @DynamoDBVersionAttribute which is provided by the aws SDK for this purpose.

Non-native it works as expected (version is set to 1 on insertion, then incremented with each update). Native a ConditionalCheckFailedException is thrown while inserting a new entity.

Based on https://github.com/quarkusio/quarkus-quickstarts/tree/main/amazon-dynamodb-quickstart and https://quarkiverse.github.io/quarkiverse-docs/quarkus-amazon-services/dev/amazon-dynamodb.html#_dynamodb_enhanced_client I prepared a sample project to reproduce the issue.

You can find the sample project at https://github.com/dagrammy/DynamoDbVersionAttribute-test.

./mvnw install -> BUILD SUCCESS ./mvnw install -Dnative -> BUILD FAILURE

Complete log: https://github.com/dagrammy/DynamoDbVersionAttribute-test/blob/main/README.md

 HTTP Request to /fruits failed, error id: 641e581d-abc1-4585-8ddb-bec115b262b3-1: software.amazon.awssdk.services.dynamodb.model.ConditionalCheckFailedException: The conditional request failed (Service: DynamoDb, Status Code: 400, Request ID: ODARM82TCOGF31YLKRI9QIU57OGW2SNPZ1DJTCVW9PFX3NY3D74C)
    at software.amazon.awssdk.core.internal.http.CombinedResponseHandler.handleErrorResponse(CombinedResponseHandler.java:125)
    at software.amazon.awssdk.core.internal.http.CombinedResponseHandler.handleResponse(CombinedResponseHandler.java:82)
    at software.amazon.awssdk.core.internal.http.CombinedResponseHandler.handle(CombinedResponseHandler.java:60)
    at software.amazon.awssdk.core.internal.http.CombinedResponseHandler.handle(CombinedResponseHandler.java:41)
    at software.amazon.awssdk.core.internal.http.pipeline.stages.HandleResponseStage.execute(HandleResponseStage.java:40)
    at software.amazon.awssdk.core.internal.http.pipeline.stages.HandleResponseStage.execute(HandleResponseStage.java:30)
    at software.amazon.awssdk.core.internal.http.pipeline.RequestPipelineBuilder$ComposingRequestPipelineStage.execute(RequestPipelineBuilder.java:206)
    at software.amazon.awssdk.core.internal.http.pipeline.stages.ApiCallAttemptTimeoutTrackingStage.execute(ApiCallAttemptTimeoutTrackingStage.java:73)
    at software.amazon.awssdk.core.internal.http.pipeline.stages.ApiCallAttemptTimeoutTrackingStage.execute(ApiCallAttemptTimeoutTrackingStage.java:42)
    at software.amazon.awssdk.core.internal.http.pipeline.stages.TimeoutExceptionHandlingStage.execute(TimeoutExceptionHandlingStage.java:78)
    at software.amazon.awssdk.core.internal.http.pipeline.stages.TimeoutExceptionHandlingStage.execute(TimeoutExceptionHandlingStage.java:40)
    at software.amazon.awssdk.core.internal.http.pipeline.stages.ApiCallAttemptMetricCollectionStage.execute(ApiCallAttemptMetricCollectionStage.java:50)
    at software.amazon.awssdk.core.internal.http.pipeline.stages.ApiCallAttemptMetricCollectionStage.execute(ApiCallAttemptMetricCollectionStage.java:36)

Is there a way to use the opmtistic locking feature in native builds?

Thanks in advance!

gsmet commented 1 year ago

Could you try adding @RegisterForReflection(targets = VersionRecordAttributeTags.class) to any of your application class and see if it fixes the issue?

If so, we will register it for reflection automatically.

gsmet commented 1 year ago

Ah, you were kind enough to provide a reproducer.

I couldn't make it to work with the following patch:

diff --git a/dynamodb-enhanced/deployment/src/main/java/io/quarkus/amazon/dynamodb/enhanced/deployment/DynamodbEnhancedProcessor.java b/dynamodb-enhanced/deployment/src/main/java/io/quarkus/amazon/dynamodb/enhanced/deployment/DynamodbEnhancedProcessor.java
index a808242..6726f3d 100644
--- a/dynamodb-enhanced/deployment/src/main/java/io/quarkus/amazon/dynamodb/enhanced/deployment/DynamodbEnhancedProcessor.java
+++ b/dynamodb-enhanced/deployment/src/main/java/io/quarkus/amazon/dynamodb/enhanced/deployment/DynamodbEnhancedProcessor.java
@@ -24,6 +24,8 @@ import io.quarkus.deployment.builditem.nativeimage.ReflectiveClassBuildItem;
 import io.quarkus.deployment.pkg.steps.NativeBuild;
 import io.quarkus.gizmo.Gizmo;
 import software.amazon.awssdk.enhanced.dynamodb.DefaultAttributeConverterProvider;
+import software.amazon.awssdk.enhanced.dynamodb.internal.extensions.AutoGeneratedTimestampRecordAttributeTags;
+import software.amazon.awssdk.enhanced.dynamodb.internal.extensions.VersionRecordAttributeTags;
 import software.amazon.awssdk.enhanced.dynamodb.internal.mapper.BeanTableSchemaAttributeTags;
 import software.amazon.awssdk.enhanced.dynamodb.mapper.BeanTableSchema;
 import software.amazon.awssdk.enhanced.dynamodb.mapper.annotations.DynamoDbBean;
@@ -52,7 +54,7 @@ public class DynamodbEnhancedProcessor {
                 .getAnnotations(DotName.createSimple(DynamoDbBean.class.getName()))) {
             ClassInfo classInfo = i.target().asClass();
             reflectiveClass.produce(
-                    new ReflectiveClassBuildItem(true, false, classInfo.name().toString()));
+                    new ReflectiveClassBuildItem(true, true, classInfo.name().toString()));
         }

         // Register classes which are used by BeanTableSchema but are not found by the classloader
@@ -60,6 +62,10 @@ public class DynamodbEnhancedProcessor {
                 new ReflectiveClassBuildItem(true, false, DefaultAttributeConverterProvider.class.getName()));
         reflectiveClass.produce(
                 new ReflectiveClassBuildItem(true, false, BeanTableSchemaAttributeTags.class.getName()));
+        reflectiveClass.produce(
+                new ReflectiveClassBuildItem(true, false, AutoGeneratedTimestampRecordAttributeTags.class.getName()));
+        reflectiveClass.produce(
+                new ReflectiveClassBuildItem(true, false, VersionRecordAttributeTags.class.getName()));

     }

Not exactly sure what's going on but my guess is that it will require adding some debug logging to the AWS library to understand what's going wrong in native.

@hamburml @Nithanim would you be interested in having a closer look?

dagrammy commented 1 year ago

Could you try adding @RegisterForReflection(targets = VersionRecordAttributeTags.class) to any of your application class and see if it fixes the issue?

If so, we will register it for reflection automatically.

Yes, we already registered some classes for reflection in the sample project. But unfortunately without any success.

https://github.com/dagrammy/DynamoDbVersionAttribute-test/blob/main/src/main/java/org/acme/dynamodb/ReflectionConfig.java

@RegisterForReflection(targets = {
    software.amazon.awssdk.enhanced.dynamodb.extensions.AtomicCounterExtension.class,
    software.amazon.awssdk.enhanced.dynamodb.extensions.WriteModification.class,
    software.amazon.awssdk.enhanced.dynamodb.extensions.VersionedRecordExtension.class,
    software.amazon.awssdk.enhanced.dynamodb.extensions.ReadModification.class,
    software.amazon.awssdk.enhanced.dynamodb.extensions.AutoGeneratedTimestampRecordExtension.class,
    software.amazon.awssdk.enhanced.dynamodb.extensions.annotations.DynamoDbVersionAttribute.class,
    software.amazon.awssdk.enhanced.dynamodb.extensions.annotations.DynamoDbAtomicCounter.class,
    software.amazon.awssdk.enhanced.dynamodb.extensions.annotations.DynamoDbAutoGeneratedTimestampAttribute.class,
    software.amazon.awssdk.enhanced.dynamodb.internal.extensions.VersionRecordAttributeTags.class,
    software.amazon.awssdk.enhanced.dynamodb.internal.extensions.DefaultDynamoDbExtensionContext.class,
    software.amazon.awssdk.enhanced.dynamodb.internal.extensions.ChainExtension.class,
    software.amazon.awssdk.enhanced.dynamodb.internal.extensions.AtomicCounterTag.class,
    software.amazon.awssdk.enhanced.dynamodb.internal.extensions.AutoGeneratedTimestampRecordAttributeTags.class
}, ignoreNested = false)
hamburml commented 1 year ago

@hamburml @Nithanim would you be interested in having a closer look?

I do not have the time right now. I'll try it at the weekend.

Nithanim commented 1 year ago

[...] @Nithanim would you be interested in having a closer look?

I'll try next weekend.

But sadly no promises since I completely switched to Azure which uses up all my brain-power. I envy you guys being in AWS :(

Nithanim commented 1 year ago

So... I lied. I should have gone to bed but went down the rabbit hole instead. Nothing precise yet but I have a long ramble about what I checked and found out; including the path I took.

If you do not want to dig into the problem yourself, the following text is useless to you!



For anyone who wants to dig into stuff like this: (The text is not proof-read and it is about 01:00 in the morning...)

Normally, an app that has problems with native-image crashes at a very specific point which most likely is the area you have to apply a patch. An example would be the core fix for ddb-enhanced that (if I remember correctly) works around the not implemented feature of runtime-defined lambdas. This crash is at compile time when trying to create this lambda. Another example would be a successful compile but crasing at runtime while accessing stuff via reflection. This is also very obvious because you have the direct point of crashing.

The current issue here is a special one. It compiles and it runs in native-mode, but it just does not work. I personally haven't seen that. The server does not like our request. In a simple world, we would have the raw HTTP communication. There might be ways, but I did not want to screw around to get that so I jumped into the code instead.

The obvious problem is that we cannot just debug the problem since in JVM it works fine. Therefore we have to search indirectly for the problem. It is exremely helpful to have a minimal, working example with tests (thank you!). Also, we know that it is a problem with @DynamoDbVersionAttribute so we use our IDE and jump there. It's just an annotation, so we let it list all usages. We quickly come to @VersionedRecordExtension.

After a quick scrolling though, we find that there is not that much code in there. But both the modifyMetadata and beforeWrite sound very helpful. I don't know (yet) what modifyMetadata does, but beforeWrite sound promising to be responsible for changes to the request to the remote server.

Since out main target with native image is reflective stuff, I made a quick check for that but nothing directly obvious in there. I have no idea what it does and the javadoc is helpful but I am tired, I just set a breakpoint and let the test run in debug mode. Examining the callstack from our service slowly to the current frame, I get some insight where it gets through. I also stepped though at some point but no obvious native-image-braking stuff.

Randomly, I came across software.amazon.awssdk.enhanced.dynamodb.internal.operations.CommonOperation#execute. For most people that would be uninteresting but this is a very lucky find because we have an excellent breakpoint candidate where we have the request and the response directly there. No need for scavanging around for that!

Our main goal is still to find out why the native-image does not work and it must be in the request. So I quickly tested the request#toString() and it makes a nice print:

PutItemRequest(TableName=QuarkusFruits, Item={name=AttributeValue(S=Cherry), description=AttributeValue(S=description-Cherry), version=AttributeValue(N=1)}, ConditionExpression=attribute_not_exists(#AMZN_MAPPED_version), ExpressionAttributeNames={#AMZN_MAPPED_version=version})

Very helpful!

Problem though: how to let it print in native-image since it is compiled in the code. You could program a detour liek other fixes but that would be too much work. Other way: replace the code. How? We get the version of the sdk from the maven deps (2.17.291), clone the aws sdk, checkout this specific version tag, add like System.out.println(request.toString()); and mvn install.

Simple? Wrong: Checkstyle and spotbugs prevent but after a quick google: -Dspotbugs.skip=true -DskipTests=true. Adn now we have a modified veersion in our local maven repo. Everything locally executed (and built) now uses our modifed dependency.

We let it test both JVM and native: mvn verify -Pnative. In JVM we get:

PutItemRequest(TableName=QuarkusFruits, Item={name=AttributeValue(S=Cherry), description=AttributeValue(S=description-Cherry), version=AttributeValue(N=1)}, ConditionExpression=attribute_not_exists(#AMZN_MAPPED_version), ExpressionAttributeNames={#AMZN_MAPPED_version=version})
PutItemResponse()
PutItemRequest(TableName=QuarkusFruits, Item={name=AttributeValue(S=Pear), description=AttributeValue(S=description-Pear), version=AttributeValue(N=1)}, ConditionExpression=attribute_not_exists(#AMZN_MAPPED_version), ExpressionAttributeNames={#AMZN_MAPPED_version=version})
PutItemResponse()
GetItemRequest(TableName=QuarkusFruits, Key={name=AttributeValue(S=Cherry)})
GetItemResponse(Item={name=AttributeValue(S=Cherry), description=AttributeValue(S=description-Cherry), version=AttributeValue(N=1)})
GetItemRequest(TableName=QuarkusFruits, Key={name=AttributeValue(S=Pear)})
GetItemResponse(Item={name=AttributeValue(S=Pear), description=AttributeValue(S=description-Pear), version=AttributeValue(N=1)})

and for native image:

PutItemRequest(TableName=QuarkusFruits, Item={name=AttributeValue(S=Cherry), description=AttributeValue(S=description-Cherry), version=AttributeValue(N=1)}, ConditionExpression=#AMZN_MAPPED_version = :old_version_value, ExpressionAttributeNames={#AMZN_MAPPED_version=version}, ExpressionAttributeValues={:old_version_value=AttributeValue(N=0)})
2022-12-18 23:48:39,055 ERROR [io.qua.ver.htt.run.QuarkusErrorHandler] (executor-thread-0) HTTP Request to /fruits failed, error id: 365d4c2b-31af-44f1-a314-4f78f66c277d-1: software.amazon.awssdk.services.dynamodb.model.ConditionalCheckFailedException: The conditional request failed (Service: DynamoDb, Status Code: 400, Request ID: O6S3KIR9QT5O70Y7Z4BBYYHKSCZ1NCBFF6VABTS872VCY2MEQ4E1)

This should be identical, but it is not! Jumping back to the beforeWrite, we have a pretty big if and also the expressions are built there. See that attribute_not_exists? This should be the condition but is is the one from the else branch.

Intermediate conclusion: The condition of the if evaluates wrong. Why? Who knows. Time to add a print there to confirm for sure and then to find out why it is wrong.

Thanks for listening to my TED-talk!

Maybe I am completely wrong or maybe not but good luck! That's all from me for today.

dagrammy commented 1 year ago

Thanks @Nithanim for your work and your TED talk. No matter how interesting but annoying I consider the problem, your TED Talk made me laugh. 😄

I also went down this rabbit hole on Friday, but not as deep as you.

After the terrific groundwork, I dove even deeper into the hole and tried to find the problem....

Like you wrote, the problem arises with the if in VersionedRecordExtension.

if (!existingVersionValue.isPresent() || isNullAttributeValue(existingVersionValue.get())) { https://github.com/aws/aws-sdk-java-v2/blob/master/services-custom/dynamodb-enhanced/src/main/java/software/amazon/awssdk/enhanced/dynamodb/extensions/VersionedRecordExtension.java#L112

So whats the value of existingVersionValue?

JVM: empty Optional

Native: AttributeValue(N=0)

Now if you go further up the chain, you will eventually get to the line Map<String, AttributeValue> itemMap = tableSchema.itemToMap(item, alwaysIgnoreNulls); in the PutItemOperation class.

If you output the values before and after the row:

JVM:

Fruit{name='Cherry', description='description-Cherry', version=null}
{name=AttributeValue(S=Cherry), description=AttributeValue(S=description-Cherry)}

Native:

Fruit{name='Cherry', description='description-Cherry', version=null}
{name=AttributeValue(S=Cherry), description=AttributeValue(S=description-Cherry), version=AttributeValue(N=0)}

Time to dig even deeper!

The itemToMap method is implemented in StaticImmutableTableSchema https://github.com/aws/aws-sdk-java-v2/blob/master/services-custom/dynamodb-enhanced/src/main/java/software/amazon/awssdk/enhanced/dynamodb/mapper/StaticImmutableTableSchema.java#L502

Again, the output of the following statement:

AttributeValue attributeValue = attributeMapper.attributeGetterMethod().apply(item);

JVM: AttributeValue(NUL=true)

Native: AttributeValue(N=0)

Time for some "serious" work. 😉 To be continued....

EDIT:

This is the function invoked (ResolvedImmutableAttribute):

Function<T, AttributeValue> getAttributeValueWithTransform = item -> {
            R value = immutableAttribute.getter().apply(item);
            return value == null ? nullAttributeValue() : attributeType.objectToAttributeValue(value);
        };

immutableAttribute.getter().apply(item); returns 0 native but null in JVM. Both times the "real getter" is invoked and returns null

dagrammy commented 1 year ago

It's late, I'm tired but still in the rabbit hole....and somehow arrived back at the start. 😉

https://github.com/quarkiverse/quarkus-amazon-services/blob/main/dynamodb-enhanced/deployment/src/main/java/io/quarkus/amazon/dynamodb/enhanced/deployment/DynamodbEnhancedProcessor.java#L102

                    } else if (name.equals("getterForProperty")) {
                        return new ReplaceMethodBody(
                                originalMethodVisitor,
                                getMaxLocals(descriptor),
                                visitor -> {
                                    visitor.visitCode();
                                    visitor.visitVarInsn(Opcodes.ALOAD, 0);
                                    visitor.visitVarInsn(Opcodes.ALOAD, 1);
                                    Type type = Type.getType(descriptor);
                                    visitor.visitMethodInsn(
                                            Opcodes.INVOKESTATIC, TARGET_METHOD_OWNER, name, type.getDescriptor(), false);
                                    visitor.visitInsn(Opcodes.ARETURN);
                                });
.....

If I understand it correctly (and honestly I only partially do right now) at this point methods are replaced for native builds which are generated in the AWS SDK during runtime.

Is it possible that there may be issues with autoboxing/conversion between Long and long? (but autoboxing a null value should cause a NPE) Does it possibly nonetheless explain that Long => null becomes long => 0?

hamburml commented 1 year ago

I am sorry, I still haven't had any time :(

If I understand it correctly (and honestly I only partially do right now) at this point methods are replaced for native builds which are generated in the AWS SDK during runtime.

It is always replaced, not only for native builds.

If you look at https://github.com/quarkiverse/quarkus-amazon-services/blob/main/dynamodb-enhanced/deployment/src/main/java/io/quarkus/amazon/dynamodb/enhanced/deployment/DynamodbEnhancedProcessor.java#L44 this is only done for nativeBuilds. That was the main issue why it didn't work in native mode because of runtime generated lambdas.

We replace a lambda method with a plain old method https://github.com/quarkiverse/quarkus-amazon-services/blob/main/dynamodb-enhanced/runtime/src/main/java/io/quarkus/amazon/dynamodb/enhanced/runtime/BeanTableSchemaSubstitutionImplementation.java#L29

Does it possibly nonetheless explain that Long => null becomes long => 0?

Hm... I do not think that graalvm converts object-types to their primitive counterparts. If you like you could try to check the parameters given to the method here (https://github.com/quarkiverse/quarkus-amazon-services/blob/main/dynamodb-enhanced/deployment/src/main/java/io/quarkus/amazon/dynamodb/enhanced/deployment/DynamodbEnhancedProcessor.java#L97). You should get the arguments via Type.getMethodType(...). See here on stackoverflow. https://stackoverflow.com/questions/67952646/how-to-get-count-and-types-of-method-parameters-using-org-objectweb-asm-methodvi

I hope this helps.

dagrammy commented 1 year ago

I am sorry, I still haven't had any time :(

If I understand it correctly (and honestly I only partially do right now) at this point methods are replaced for native builds which are generated in the AWS SDK during runtime.

It is always replaced, not only for native builds.

Thanks for the clarification. I missed that one and made a wrong assumption.

Does it possibly nonetheless explain that Long => null becomes long => 0?

Hm... I do not think that graalvm converts object-types to their primitive counterparts. If you like you could try to check the parameters given to the method here (https://github.com/quarkiverse/quarkus-amazon-services/blob/main/dynamodb-enhanced/deployment/src/main/java/io/quarkus/amazon/dynamodb/enhanced/deployment/DynamodbEnhancedProcessor.java#L97). You should get the arguments via Type.getMethodType(...). See here on stackoverflow. https://stackoverflow.com/questions/67952646/how-to-get-count-and-types-of-method-parameters-using-org-objectweb-asm-methodvi

I hope this helps.

I also can not explain why Graal should make such a transformation at this point.

Thank you also for the hints where I can continue next. I hope that tomorrow I will be able to take a closer look at the mentioned places.

dagrammy commented 1 year ago

Unfortunately I didn't have much time today. But I have included debug statements in BeanTableSchemaSubstitutionImplementation / GetterWrapper#apply and the entity.

try {
  R retVal = (R) mh.invoke(t);
  System.out.println("GetterWrapper returns: " + retVal);
  if (retVal != null) {
      System.out.println("GetterWrapper returns instance of: " + retVal.getClass().getName());
  }
  return (R) mh.invoke(t);
  ....

and

  @DynamoDbVersionAttribute
  public Long getVersion() {
      System.out.println("getVersion() called and returns: " + version);
      return version;
  }

The result: JVM:

getVersion() called and returns: null
GetterWrapper returns: null

Native:

getVersion() called and returns: null
GetterWrapper returns: 0
GetterWrapper returns instance of: java.lang.Long

Then I tried to figure out what happens with the parameters @ https://github.com/quarkiverse/quarkus-amazon-services/blob/main/dynamodb-enhanced/deployment/src/main/java/io/quarkus/amazon/dynamodb/enhanced/deployment/DynamodbEnhancedProcessor.java#L102

So I added some debug statements (thx to your SO link):

    Type methodType = Type.getMethodType(descriptor);
    int sizes = methodType.getArgumentsAndReturnSizes();
    System.out.println("return type: "
            + methodType.getReturnType().getClassName() + " (" + (sizes & 3) + ')');

    Type[] argTypes = methodType.getArgumentTypes();
    System.out.println(argTypes.length + " arguments (" + (sizes >> 2) + ')');
    for (int ix = 0; ix < argTypes.length; ix++) {
        System.out.println("arg" + ix + ": " + argTypes[ix].getClassName());
    }

Output:

return type: java.util.function.Function (1)
2 arguments (3)
arg0: java.beans.PropertyDescriptor
arg1: java.lang.Class

But these are just the return type & parameters (PropertyDescriptor propertyDescriptor, Class<T> beanClass) of BeanTableSchema#getterForProperty.

I'm at my wit's end (or...attention, very poor 1:1 translation from german: "I am at the end with my latin" 😉)

So I am stuck right now and don't know how to get even deeper.

I will nevertheless continue to try my best to track down the reason for this strange behavior.

Nithanim commented 1 year ago

It is very kind of you that you tracked it all the way back to the MethodHandles, thank you! It gets a lot easier when someone else does the work for you :stuck_out_tongue: and you can trim everything away and get a smaller example project: https://github.com/Nithanim/quarkus-native-bug-test

So for some weird reason, the MethodHandle for Long is broken. The problem is that there is not a whole lot of things I can think of trying right now. It is pretty much the border to the arcane magic of native image.

Questions:

* I don't want github to cross-link the issues (yet).

Sorry, that I don't have a lot of value to add yet.

I'm at my wit's end (or...attention, very poor 1:1 translation from german: "I am at the end with my latin" wink)

At work I am doing something with "underwriting"s and "bookholding". Hah! :smile:

dagrammy commented 1 year ago

It is very kind of you that you tracked it all the way back to the MethodHandles, thank you! It gets a lot easier when someone else does the work for you 😛 and you can trim everything away and get a smaller example project: https://github.com/Nithanim/quarkus-native-bug-test

You are welcome! 😊 Well, it's a team effort! 😉 So thank you for example project!

So for some weird reason, the MethodHandle for Long is broken. The problem is that there is not a whole lot of things I can think of trying right now. It is pretty much the border to the arcane magic of native image.

I took your example project and stripped it down even further (no Quarkus, no maven....just simple javac, java and native-image). https://github.com/dagrammy/graal-native-bug-test

Questions:

  • Only Long specifically, or other primitive wrappers too?

It's reproducible using Integer and Boolean too.

  • Does Quarkus do some magic? => Even more minimal project without Quarkus; call native-image manually. It would be pretty weird to have such an obvious native-image bug.

I don't think so...

  • wtf is going on?

This is the right question of all questions. 😆

  • Is there a difference between Mandrel and GraalVM? (Most likely not, but...)

  • Maybe there is something in the issue tracker, like https://github.com/oracle/graal/issues /5209 *. Seems related at least.

  • ...

  • I don't want github to cross-link the issues (yet).

I also did some research in the issue tracker, but was not able to find sth. So I decided to open an issue using the sample project as reproducer.

https://github.com/oracle/graal/issues/5672

Sorry, that I don't have a lot of value to add yet.

I'm at my wit's end (or...attention, very poor 1:1 translation from german: "I am at the end with my latin" wink)

At work I am doing something with "underwriting"s and "bookholding". Hah! 😄

To quote a colleague. "I should have learned something serious. Carpenter or something like that..."

With this in mind, I wish everyone happy holidays.

Nithanim commented 1 year ago

I took your example project and stripped it down even further (no Quarkus, no maven....just simple javac, java and native-image). https://github.com/dagrammy/graal-native-bug-test

You really took "minimal" to the next level, even stripping out poor maven :cry:
Really nice, thank you!

It's reproducible using Integer and Boolean too. This is the right question of all questions. laughing

This is so weird. I am really eager to find out what is going on.

I also did some research in the issue tracker, but was not able to find sth. So I decided to open an issue using the sample project as reproducer.

https://github.com/oracle/graal/issues/ 5672 (also messed up the link intentionally)

Perfect! You are doing all the complicated stuff :smile: You could have let Github link the issues because it is directly related but ¯\_(ツ)_/¯

To quote a colleague. "I should have learned something serious. Carpenter or something like that..."

I often say I really do should become "Bauer im Hinterland" (like "farmer in the outbacks") to leave all weird crap behind.

With this in mind, I wish everyone happy holidays.

Happy holidays to you too!

dagrammy commented 1 year ago

You really took "minimal" to the next level, even stripping out poor maven 😢 Really nice, thank you!

I also couldn't remember the last time I typed javac into a terminal. 😄

This is so weird. I am really eager to find out what is going on.

Me too...me too!!

Perfect! You are doing all the complicated stuff 😄 You could have let Github link the issues because it is directly related but ¯_(ツ)_/¯

Fixed the link so the issue should be linked (if that works for an edit).

I often say I really do should become "Bauer im Hinterland" (like "farmer in the outbacks") to leave all weird crap behind.

🤣

After some thinking, I decided: Why not go a little old school, bypass MethodHandle and try Method#invoke directly.

And et voilà...it works! https://github.com/dagrammy/graal-native-bug-test/blob/extended/MethodHandleTest.java#L31

Method getValue = beanClass.getDeclaredMethod("getValue");
Long v = (Long) getValue.invoke(bean);
System.out.println(">>>>> " + v + " <<<<<");

returns null for native and JVM!

I also added some simple test for the setter MethodHandles and they seem to work with native. https://github.com/dagrammy/graal-native-bug-test/blob/extended/SetterTest.java

So I came up with the following PR: https://github.com/quarkiverse/quarkus-amazon-services/pull/443

I tested the dynamodb-enhanced Quarkus integration with my initial reproducer at https://github.com/dagrammy/DynamoDbVersionAttribute-test and it works (version attribute is set to 1 on insert and incremented afterwards)

What do you think of the PR respectively of the idea?

hamburml commented 1 year ago

Welcome to 2023 :)

I hope the graalvm guys will soon reply to your issue. It is great that you found the problem. Ideally we would not need your workaround but instead get a new graalvm version.

What do you think of the PR respectively of the idea?

I would suggest adding some tests for the use of DynamoDBVersionAttribute. When we use a new graalvm version and set it back to MethodHandle, it should still work and with tests we can ensure that.

dagrammy commented 1 year ago

Welcome to 2023 :)

Happy new year too! 🥳

I hope the graalvm guys will soon reply to your issue. It is great that you found the problem. Ideally we would not need your workaround but instead get a new graalvm version.

I also hope that the graalvm team will address the issue soon. That would be the best solution, of course.

I would suggest adding some tests for the use of DynamoDBVersionAttribute. When we use a new graalvm version and set it back to MethodHandle, it should still work and with tests we can ensure that.

That's an excellent idea. To be honest, I hadn't even seen the tests for the enhanced client. 😉 But now I have discovered the integrationtests.

I updated the existing test classes and added new fields with annotations @DynamoDbVersionAttribute and @DynamoDbAutoGeneratedTimestampAttribute. I also extended the test as @QuarkusIntegrationTest so the test is run in native mode. A new commit was pushed to the PR.

With the latest version the test fails, with the PR workaround the test succeeds.

gsmet commented 1 year ago

1.5.0 has hit Central with the fix contributed by @dagrammy