korlov42 commented on code in PR #6593:
URL: https://github.com/apache/ignite-3/pull/6593#discussion_r2354496863
##########
modules/sql-engine/src/main/java/org/apache/ignite/internal/sql/engine/util/cache/Cache.java:
##########
@@ -93,4 +95,12 @@ public interface Cache<K, V> {
* @return The number of entries in the cache.
*/
int size();
+
Review Comment:
please revert all changes in this package. You don't need them
##########
modules/sql-engine/src/integrationTest/java/org/apache/ignite/internal/sql/engine/statistic/ItStatisticTest.java:
##########
@@ -44,6 +46,11 @@ void afterAll() {
sql("DROP TABLE IF EXISTS t");
}
+ @Override
+ protected int initialNodes() {
Review Comment:
Doesn't look like speed up to me, thus let's revert this change
##########
modules/sql-engine/src/main/java/org/apache/ignite/internal/sql/engine/prepare/PrepareServiceImpl.java:
##########
@@ -415,23 +502,24 @@ private CompletableFuture<QueryPlan> prepareExplain(
}
return result.thenApply(plan -> {
- assert plan instanceof ExplainablePlan : plan == null ? "<null>" :
plan.getClass().getCanonicalName();
+ QueryPlan plan0 = plan.queryPlan;
Review Comment:
please avoid using Ignite2-style names. You've changed original variable --
just rename it to better reflect semantic
##########
modules/sql-engine/src/main/java/org/apache/ignite/internal/sql/engine/prepare/PrepareService.java:
##########
@@ -48,4 +48,6 @@ public interface PrepareService extends LifecycleAware {
default CompletableFuture<Void> invalidateCache(Set<String> tableNames) {
return CompletableFutures.nullCompletedFuture();
}
+
+ void updatePlans(int tableId);
Review Comment:
this must not be part of the interface
##########
modules/sql-engine/src/main/java/org/apache/ignite/internal/sql/engine/prepare/CacheKey.java:
##########
@@ -77,6 +94,9 @@ public boolean equals(Object o) {
if (!schemaName.equals(cacheKey.schemaName)) {
return false;
}
+ if (queryType != cacheKey.queryType) {
+ return false;
Review Comment:
why do you need query type to be part of equals+hashCode contract?
##########
modules/sql-engine/src/main/java/org/apache/ignite/internal/sql/engine/prepare/PrepareServiceImpl.java:
##########
@@ -231,12 +262,67 @@ public void start() {
metricManager.enable(PLANNING_EXECUTOR_SOURCE_NAME);
IgnitePlanner.warmup();
+
+ ScheduledExecutorService planUpdater =
Executors.newSingleThreadScheduledExecutor(
+ IgniteThreadFactory.create(nodeName, "sql-query-plan-refresh",
true, LOG)
+ );
Review Comment:
we already have a few hundreds of "lightweight" threads, I would avoid
adding more
--
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]
