ppalaga commented on a change in pull request #1902:
URL: https://github.com/apache/camel-quarkus/pull/1902#discussion_r503720527
##########
File path:
extensions/leveldb/deployment/src/main/java/org/apache/camel/quarkus/component/leveldb/deployment/LeveldbProcessor.java
##########
@@ -16,31 +16,41 @@
*/
package org.apache.camel.quarkus.component.leveldb.deployment;
+import io.quarkus.deployment.annotations.BuildProducer;
import io.quarkus.deployment.annotations.BuildStep;
-import io.quarkus.deployment.annotations.ExecutionTime;
-import io.quarkus.deployment.annotations.Record;
import io.quarkus.deployment.builditem.FeatureBuildItem;
-import io.quarkus.deployment.pkg.steps.NativeBuild;
-import org.apache.camel.quarkus.core.JvmOnlyRecorder;
-import org.jboss.logging.Logger;
+import io.quarkus.deployment.builditem.nativeimage.ReflectiveClassBuildItem;
+import
io.quarkus.deployment.builditem.nativeimage.RuntimeInitializedClassBuildItem;
+import org.apache.camel.support.DefaultExchangeHolder;
class LeveldbProcessor {
- private static final Logger LOG = Logger.getLogger(LeveldbProcessor.class);
private static final String FEATURE = "camel-leveldb";
@BuildStep
FeatureBuildItem feature() {
return new FeatureBuildItem(FEATURE);
}
- /**
- * Remove this once this extension starts supporting the native mode.
- */
- @BuildStep(onlyIf = NativeBuild.class)
- @Record(value = ExecutionTime.RUNTIME_INIT)
- void warnJvmInNative(JvmOnlyRecorder recorder) {
- JvmOnlyRecorder.warnJvmInNative(LOG, FEATURE); // warn at build time
- recorder.warnJvmInNative(FEATURE); // warn at runtime
+ @BuildStep
+ ReflectiveClassBuildItem registerForReflection() {
+ return new ReflectiveClassBuildItem(false, false, new String[] {
+ org.iq80.leveldb.impl.Iq80DBFactory.class.getName(),
+ org.apache.camel.support.DefaultExchangeHolder.class.getName()
+ });
+ }
+
+ @BuildStep
+ ReflectiveClassBuildItem registerForReflectionWithFields() {
+ return new ReflectiveClassBuildItem(false, true, new String[] {
+ DefaultExchangeHolder.class.getName(),
Review comment:
Hm... you even register the DefaultExchangeHolder twice. I guess this
one is right and the above is wrong? But once again is registering
org.apache.camel.support.DefaultExchangeHolder really necessary?
##########
File path: extensions/leveldb/runtime/src/main/doc/limitations.adoc
##########
@@ -0,0 +1,5 @@
+In native mode extension uses port of the LevelDB written in Java
(https://github.com/dain/leveldb#leveldb-in-java[documentation]),
+which is within 10% of the performance of the C++ original. Usage of native
implementation will be investigated as a new issue.
Review comment:
```suggestion
which is within 10% of the performance of the C++ original. Please upvote
link-to-new-issue[this issue] if you do not like the present state.
```
##########
File path:
integration-tests/leveldb/src/main/java/org/apache/camel/quarkus/component/leveldb/it/LeveldbRouteBuilder.java
##########
@@ -0,0 +1,102 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.camel.quarkus.component.leveldb.it;
+
+import java.util.concurrent.TimeUnit;
+import java.util.concurrent.atomic.AtomicInteger;
+
+import org.apache.camel.AggregationStrategy;
+import org.apache.camel.Exchange;
+import org.apache.camel.builder.RouteBuilder;
+import org.apache.camel.component.leveldb.LevelDBAggregationRepository;
+
+public class LeveldbRouteBuilder extends RouteBuilder {
+ public static final String DIRECT_START = "direct:start";
+ public static final String DIRECT_START_WITH_FAILURE =
"direct:startWithFailure";
+ public static final String DIRECT_START_DEAD_LETTER =
"direct:startDeadLetter";
+ public static final String MOCK_AGGREGATED = "mock:aggregated";
+ public static final String MOCK_RESULT = "mock:result";
+ public static final String MOCK_DEAD = "mock:dead";
+ public static final String DATA_FOLDER = "target/data";
+
+ private static AtomicInteger counter = new AtomicInteger(0);
+
+ @Override
+ public void configure() throws Exception {
+ LevelDBAggregationRepository repo = new
LevelDBAggregationRepository("repo", DATA_FOLDER + "leveldb.dat");
+
+ from(DIRECT_START)
+ .aggregate(header("id"), new MyAggregationStrategy())
+ .completionSize(7).aggregationRepository(repo)
+ .to(MOCK_RESULT);
+
+ LevelDBAggregationRepository repoWithFailure = new
LevelDBAggregationRepository("repoWithFailure",
+ DATA_FOLDER + "leveldbWithFailure.dat");
+
+ repoWithFailure.setUseRecovery(true);
+ repoWithFailure.setRecoveryInterval(500, TimeUnit.MILLISECONDS);
+
+ from(DIRECT_START_WITH_FAILURE)
+ .aggregate(header("id"), new MyAggregationStrategy())
+ .completionSize(7).aggregationRepository(repoWithFailure)
+ .to(MOCK_AGGREGATED)
+ .delay(1000)
Review comment:
Why do we need the delay here?
##########
File path:
integration-tests/leveldb/src/test/java/org/apache/camel/quarkus/component/leveldb/it/LeveldbTest.java
##########
@@ -0,0 +1,117 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.camel.quarkus.component.leveldb.it;
+
+import java.io.File;
+import java.util.Arrays;
+import java.util.List;
+import java.util.Map;
+
+import io.quarkus.test.junit.QuarkusTest;
+import io.restassured.RestAssured;
+import io.restassured.http.ContentType;
+import io.restassured.specification.RequestSpecification;
+import org.apache.camel.Exchange;
+import org.apache.commons.io.FileUtils;
+import org.junit.jupiter.api.AfterAll;
+import org.junit.jupiter.api.Test;
+
+import static org.junit.jupiter.api.Assertions.assertEquals;
+import static org.junit.jupiter.api.Assertions.assertFalse;
+import static org.junit.jupiter.api.Assertions.assertTrue;
+
+@QuarkusTest
+class LeveldbTest {
+
+ @Test
+ public void testAggregate() {
+ Map<String, List<Map<String, Object>>> data =
testAggregate(LeveldbRouteBuilder.DIRECT_START,
+ Arrays.asList(new String[] { "S", "H", "E", "L", "D", "O", "N"
}));
Review comment:
Style tip: Arrays.asList() accepts multiarg too.
##########
File path:
extensions/leveldb/deployment/src/main/java/org/apache/camel/quarkus/component/leveldb/deployment/LeveldbProcessor.java
##########
@@ -16,31 +16,41 @@
*/
package org.apache.camel.quarkus.component.leveldb.deployment;
+import io.quarkus.deployment.annotations.BuildProducer;
import io.quarkus.deployment.annotations.BuildStep;
-import io.quarkus.deployment.annotations.ExecutionTime;
-import io.quarkus.deployment.annotations.Record;
import io.quarkus.deployment.builditem.FeatureBuildItem;
-import io.quarkus.deployment.pkg.steps.NativeBuild;
-import org.apache.camel.quarkus.core.JvmOnlyRecorder;
-import org.jboss.logging.Logger;
+import io.quarkus.deployment.builditem.nativeimage.ReflectiveClassBuildItem;
+import
io.quarkus.deployment.builditem.nativeimage.RuntimeInitializedClassBuildItem;
+import org.apache.camel.support.DefaultExchangeHolder;
class LeveldbProcessor {
- private static final Logger LOG = Logger.getLogger(LeveldbProcessor.class);
private static final String FEATURE = "camel-leveldb";
@BuildStep
FeatureBuildItem feature() {
return new FeatureBuildItem(FEATURE);
}
- /**
- * Remove this once this extension starts supporting the native mode.
- */
- @BuildStep(onlyIf = NativeBuild.class)
- @Record(value = ExecutionTime.RUNTIME_INIT)
- void warnJvmInNative(JvmOnlyRecorder recorder) {
- JvmOnlyRecorder.warnJvmInNative(LOG, FEATURE); // warn at build time
- recorder.warnJvmInNative(FEATURE); // warn at runtime
+ @BuildStep
+ ReflectiveClassBuildItem registerForReflection() {
+ return new ReflectiveClassBuildItem(false, false, new String[] {
+ org.iq80.leveldb.impl.Iq80DBFactory.class.getName(),
+ org.apache.camel.support.DefaultExchangeHolder.class.getName()
+ });
Review comment:
Style tip: the last arg of `ReflectiveClassBuildItem` constructor is
multiarg, so you could make it shorter by omitting the the array declaration
(it would compile to exactly the same bytecode):
```suggestion
return new ReflectiveClassBuildItem(false, false,
org.iq80.leveldb.impl.Iq80DBFactory.class.getName(),
org.apache.camel.support.DefaultExchangeHolder.class.getName());
```
Is org.apache.camel.support.DefaultExchangeHolder really necessary? IIRC,
there was some discussion around this recently.
##########
File path: extensions/leveldb/runtime/src/main/doc/limitations.adoc
##########
@@ -0,0 +1,5 @@
+In native mode extension uses port of the LevelDB written in Java
(https://github.com/dain/leveldb#leveldb-in-java[documentation]),
+which is within 10% of the performance of the C++ original. Usage of native
implementation will be investigated as a new issue.
+
+GraalVM does not support `ObjectOutputStream.writeObject()`
(https://github.com/oracle/graal/issues/460[issue]).
+In native mode extension uses Jackson serialization instead. This approach
could have some performance impact.
Review comment:
Looking at [this comparison from
2014](http://rick-hightower.blogspot.com/2014/04/which-is-faster-java-object.html)
the impact will most likely be be positive in most cases. I'd remove the whole
paragraph.
##########
File path: integration-tests/leveldb/src/main/resources/application.properties
##########
@@ -0,0 +1,20 @@
+## ---------------------------------------------------------------------------
+## Licensed to the Apache Software Foundation (ASF) under one or more
+## contributor license agreements. See the NOTICE file distributed with
+## this work for additional information regarding copyright ownership.
+## The ASF licenses this file to You under the Apache License, Version 2.0
+## (the "License"); you may not use this file except in compliance with
+## the License. You may obtain a copy of the License at
+##
+## http://www.apache.org/licenses/LICENSE-2.0
+##
+## Unless required by applicable law or agreed to in writing, software
+## distributed under the License is distributed on an "AS IS" BASIS,
+## WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+## See the License for the specific language governing permissions and
+## limitations under the License.
+## ---------------------------------------------------------------------------
+
+quarkus.log.category."org.apache.camel.component.leveldb".level = DEBUG
Review comment:
I do not think we want to merge it with debug on. Couldn't we comment it
at least?
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
[email protected]
