sandynz commented on code in PR #27251:
URL: https://github.com/apache/shardingsphere/pull/27251#discussion_r1266093823
##########
kernel/data-pipeline/api/pom.xml:
##########
@@ -41,6 +41,10 @@
<groupId>commons-codec</groupId>
<artifactId>commons-codec</artifactId>
</dependency>
+ <dependency>
+ <groupId>com.github.ben-manes.caffeine</groupId>
+ <artifactId>caffeine</artifactId>
+ </dependency>
Review Comment:
`PipelineImportSQLBuilder` is in pipeline `core` module, could we put the
dependency into`core` module?
##########
kernel/data-pipeline/core/src/main/java/org/apache/shardingsphere/data/pipeline/common/sqlbuilder/PipelineImportSQLBuilder.java:
##########
@@ -44,12 +46,20 @@ public final class PipelineImportSQLBuilder {
private final PipelineSQLSegmentBuilder sqlSegmentBuilder;
- private final ConcurrentMap<String, String> sqlCacheMap;
+ private final Cache<String, String> sqlCache;
- public PipelineImportSQLBuilder(final DatabaseType databaseType) {
+ public PipelineImportSQLBuilder(final DatabaseType databaseType,final
ShardingCacheOptionsConfiguration cacheOptions) {
Review Comment:
Could we define a suitable `cacheOptions` in `PipelineImportSQLBuilder`?
This is different with ShardingRouteCache
##########
kernel/data-pipeline/core/src/main/java/org/apache/shardingsphere/data/pipeline/common/sqlbuilder/PipelineImportSQLBuilder.java:
##########
@@ -61,11 +71,11 @@ public PipelineImportSQLBuilder(final DatabaseType
databaseType) {
*/
public String buildInsertSQL(final String schemaName, final DataRecord
dataRecord) {
String sqlCacheKey = INSERT_SQL_CACHE_KEY_PREFIX +
dataRecord.getTableName();
- if (!sqlCacheMap.containsKey(sqlCacheKey)) {
+ if (null!=sqlCache.getIfPresent(sqlCacheKey)) {
Review Comment:
There's some code style could be improved. Refer to [Code of Conducts](
https://shardingsphere.apache.org/community/en/involved/conduct/code/ ) for
more details, and format code.
##########
kernel/data-pipeline/core/src/main/java/org/apache/shardingsphere/data/pipeline/common/sqlbuilder/PipelineImportSQLBuilder.java:
##########
@@ -44,12 +46,20 @@ public final class PipelineImportSQLBuilder {
private final PipelineSQLSegmentBuilder sqlSegmentBuilder;
- private final ConcurrentMap<String, String> sqlCacheMap;
+ private final Cache<String, String> sqlCache;
- public PipelineImportSQLBuilder(final DatabaseType databaseType) {
+ public PipelineImportSQLBuilder(final DatabaseType databaseType,final
ShardingCacheOptionsConfiguration cacheOptions) {
dialectSQLBuilder =
DatabaseTypedSPILoader.getService(DialectPipelineSQLBuilder.class,
databaseType);
sqlSegmentBuilder = new PipelineSQLSegmentBuilder(databaseType);
- sqlCacheMap = new ConcurrentHashMap<>();
+ sqlCache = buildRouteCache(cacheOptions);
+ }
+
+ private Cache<String, String> buildRouteCache(final
ShardingCacheOptionsConfiguration cacheOptions) {
Review Comment:
1, `buildRouteCache` could be renamed, since there's no route here.
2, From `new ShardingCacheOptionsConfiguration(true, 1, 1)`, maximum size
could not be `1`.
--
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]
