gnodet commented on code in PR #25034: URL: https://github.com/apache/camel/pull/25034#discussion_r3632902751
########## components/camel-clickhouse/src/main/java/org/apache/camel/component/clickhouse/ClickHouseEndpoint.java: ########## @@ -0,0 +1,242 @@ +/* + * 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.component.clickhouse; + +import com.clickhouse.client.api.Client; +import org.apache.camel.Category; +import org.apache.camel.Consumer; +import org.apache.camel.Processor; +import org.apache.camel.Producer; +import org.apache.camel.spi.Metadata; +import org.apache.camel.spi.UriEndpoint; +import org.apache.camel.spi.UriParam; +import org.apache.camel.spi.UriPath; +import org.apache.camel.support.DefaultEndpoint; +import org.apache.camel.util.ObjectHelper; + +/** + * Interact with <a href="https://clickhouse.com/";>ClickHouse</a>, the high-performance columnar OLAP database, for + * high-throughput ingestion and OLAP queries. + */ +@UriEndpoint(firstVersion = "4.22.0", scheme = "clickhouse", title = "ClickHouse", + syntax = "clickhouse:database", category = { Category.DATABASE, Category.BIGDATA }, + producerOnly = true, headersClass = ClickHouseConstants.class) +public class ClickHouseEndpoint extends DefaultEndpoint { + + private Client client; + private boolean clientCreated; + + @UriPath + @Metadata(required = true, + description = "The ClickHouse database. A table may also be provided using the database.table syntax.") + private String database; Review Comment: The `client` field is used in a double-checked locking pattern in `getClient()` but is not declared `volatile`. Without `volatile`, the Java Memory Model does not guarantee that a second thread will see a fully-constructed `Client` object — it may observe a partially-initialized instance. ```suggestion private volatile Client client; ``` ########## docs/user-manual/modules/ROOT/pages/camel-4x-upgrade-guide-4_22.adoc: ########## @@ -217,23 +217,27 @@ to the true average throughput rather than oscillating between zero and spike va If you have tooling that consumes the JMX throughput value and expects the old instantaneous behavior, be aware that the reported value will now ramp up and decay gradually. -=== camel-azure-servicebus - Camel-managed message lock renewal +=== camel-azure-servicebus - Camel-managed message and session lock renewal Review Comment: This modification to the `camel-azure-servicebus` section (changing "message lock renewal" to "message and session lock renewal" and adding session-related content) appears unrelated to the ClickHouse component. These changes should ideally be in a separate PR to keep this one focused on the new component. ########## components/camel-clickhouse/src/main/java/org/apache/camel/component/clickhouse/ClickHouseEndpoint.java: ########## @@ -0,0 +1,242 @@ +/* + * 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.component.clickhouse; + +import com.clickhouse.client.api.Client; +import org.apache.camel.Category; +import org.apache.camel.Consumer; +import org.apache.camel.Processor; +import org.apache.camel.Producer; +import org.apache.camel.spi.Metadata; +import org.apache.camel.spi.UriEndpoint; +import org.apache.camel.spi.UriParam; +import org.apache.camel.spi.UriPath; +import org.apache.camel.support.DefaultEndpoint; +import org.apache.camel.util.ObjectHelper; + +/** + * Interact with <a href="https://clickhouse.com/";>ClickHouse</a>, the high-performance columnar OLAP database, for + * high-throughput ingestion and OLAP queries. + */ +@UriEndpoint(firstVersion = "4.22.0", scheme = "clickhouse", title = "ClickHouse", + syntax = "clickhouse:database", category = { Category.DATABASE, Category.BIGDATA }, + producerOnly = true, headersClass = ClickHouseConstants.class) +public class ClickHouseEndpoint extends DefaultEndpoint { + + private Client client; + private boolean clientCreated; + + @UriPath + @Metadata(required = true, + description = "The ClickHouse database. A table may also be provided using the database.table syntax.") + private String database; + @UriParam(description = "The target table for insert operations. Can also be given in the path as database.table" + + " or overridden per message with the CamelClickHouseTable header.") + private String table; + @UriParam(description = "The ClickHouse HTTP endpoint URL, e.g. http://localhost:8123. Required unless a shared" + + " Client bean is autowired or configured on the component.") + private String serverUrl; + @UriParam(label = "security", defaultValue = "default", description = "The username used to authenticate to ClickHouse.") + private String username = "default"; + @UriParam(label = "security", secret = true, description = "The password used to authenticate to ClickHouse.") + private String password; + @UriParam(label = "security", defaultValue = "false", + description = "Whether to connect to ClickHouse over a secure (HTTPS) connection.") + private boolean ssl; + @UriParam(defaultValue = "INSERT", description = "The operation to perform: insert, query or ping.") + private ClickHouseOperation operation = ClickHouseOperation.INSERT; + @UriParam(defaultValue = "JSONEachRow", + description = "The ClickHouse data format used for insert and query operations, e.g. JSONEachRow," + + " RowBinary, CSV, TSV or Parquet.") + private String format = "JSONEachRow"; Review Comment: The `ssl` endpoint option is declared and documented as "Whether to connect to ClickHouse over a secure (HTTPS) connection", but `getClient()` never applies it to the `Client.Builder`. A user setting `ssl=true` will silently get an insecure connection with no error or warning. The ClickHouse Java V2 client supports SSL either by using `https://` in the endpoint URL or by passing `secure=true` to `addEndpoint(Protocol, host, port, secure)`. The `getClient()` method should use the `ssl` flag to configure the appropriate protocol. ########## components/camel-clickhouse/src/main/java/org/apache/camel/component/clickhouse/ClickHouseProducer.java: ########## @@ -0,0 +1,185 @@ +/* + * 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.component.clickhouse; + +import java.io.ByteArrayInputStream; +import java.io.File; +import java.io.FileInputStream; +import java.io.InputStream; +import java.nio.charset.StandardCharsets; +import java.util.List; + +import com.clickhouse.client.api.Client; +import com.clickhouse.client.api.insert.InsertResponse; +import com.clickhouse.client.api.insert.InsertSettings; +import com.clickhouse.client.api.query.QueryResponse; +import com.clickhouse.client.api.query.QuerySettings; +import com.clickhouse.data.ClickHouseFormat; +import org.apache.camel.Exchange; +import org.apache.camel.Message; +import org.apache.camel.support.DefaultProducer; +import org.apache.camel.util.IOHelper; +import org.apache.camel.util.ObjectHelper; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; + +public class ClickHouseProducer extends DefaultProducer { + + private static final Logger LOG = LoggerFactory.getLogger(ClickHouseProducer.class); + + private final ClickHouseEndpoint endpoint; + + public ClickHouseProducer(ClickHouseEndpoint endpoint) { + super(endpoint); + this.endpoint = endpoint; + } + + @Override + public void process(Exchange exchange) throws Exception { + ClickHouseOperation operation = resolveOperation(exchange); + switch (operation) { Review Comment: The `batchSize` endpoint option is declared in `ClickHouseEndpoint` with a description about client-side batching, but it is never referenced anywhere in `ClickHouseProducer`. The `doInsert()` method streams data without any batching logic. Either implement client-side batching using `batchSize`, or remove the option to avoid misleading users. -- 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. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
