alex-plekhanov commented on code in PR #11824:
URL: https://github.com/apache/ignite/pull/11824#discussion_r1953070857
##########
modules/core/src/main/java/org/apache/ignite/internal/processors/cache/ClusterCachesInfo.java:
##########
@@ -1616,37 +1616,35 @@ else if
(!GridFunc.eqNotOrdered(desc.schema().entities(), locQryEntities))
cfg.getNearConfiguration() != null);
}
- updateRegisteredCachesIfNeeded(patchesToApply, cachesToSave,
hasSchemaPatchConflict);
+ if (!hasSchemaPatchConflict)
+ updateRegisteredCaches(patchesToApply, cachesToSave);
}
/**
- * Merging config or resaving it if it needed.
+ * Merging config, resaving it if it needed.
*
* @param patchesToApply Patches which need to apply.
* @param cachesToSave Caches which need to resave.
- * @param hasSchemaPatchConflict {@code true} if we have conflict during
making patch.
- */
- private void updateRegisteredCachesIfNeeded(Map<DynamicCacheDescriptor,
QuerySchemaPatch> patchesToApply,
- Collection<DynamicCacheDescriptor> cachesToSave, boolean
hasSchemaPatchConflict) {
- //Skip merge of config if least one conflict was found.
- if (!hasSchemaPatchConflict) {
- boolean isClusterActive = ctx.state().clusterState().active();
-
- //Merge of config for cluster only for inactive grid.
- if (!isClusterActive && !patchesToApply.isEmpty()) {
- for (Map.Entry<DynamicCacheDescriptor, QuerySchemaPatch> entry
: patchesToApply.entrySet()) {
- if (entry.getKey().applySchemaPatch(entry.getValue()))
- saveCacheConfiguration(entry.getKey());
- }
+ */
+ private void updateRegisteredCaches(
+ Map<DynamicCacheDescriptor, QuerySchemaPatch> patchesToApply,
+ Collection<DynamicCacheDescriptor> cachesToSave
+ ) {
+ // Store config only if cluster is nactive.
+ boolean isClusterActive = ctx.state().clusterState().active();
- for (DynamicCacheDescriptor descriptor : cachesToSave)
- saveCacheConfiguration(descriptor);
- }
- else if (patchesToApply.isEmpty()) {
- for (DynamicCacheDescriptor descriptor : cachesToSave)
- saveCacheConfiguration(descriptor);
+ //Merge of config for cluster only for inactive grid.
Review Comment:
Space after `//`
Incorrect comment, now we merge configs (apply patch) for any cluster state,
but store config only for active cluster
##########
modules/calcite/src/test/java/org/apache/ignite/internal/processors/query/calcite/integration/TableDdlIntegrationTest.java:
##########
@@ -995,6 +1016,51 @@ public void testPrimaryKeyInlineSize() {
"MY_TABLE", 5 + 3);
}
+ /** */
+ @Test
+ public void
testNonPersistentRejoinsWithDynamicTablesOverPredefinedCaches() throws
Exception {
+ stopAllGrids();
+
+ try {
+ persistence = false;
+
+ CacheConfiguration<?, ?> cacheCfg = new
CacheConfiguration<>("TEST_CACHE")
+ .setBackups(1)
+ .setCacheMode(CacheMode.PARTITIONED)
+ .setAtomicityMode(CacheAtomicityMode.TRANSACTIONAL)
+
.setWriteSynchronizationMode(CacheWriteSynchronizationMode.PRIMARY_SYNC);
+
+ cacheConfigurations = new CacheConfiguration<?, ?>[] {cacheCfg};
+
+ client = startGrids(3);
+
+ sql("CREATE TABLE IF NOT EXISTS TEST_TBL(ID INTEGER PRIMARY KEY,
VAL VARCHAR) WITH \"CACHE_NAME=TEST_CACHE\"");
+
+ assertEquals(0, sql("SELECT * FROM TEST_TBL").size());
+
+ int testGrid = 2;
+
+ stopGrid(testGrid);
+ startGrid(testGrid);
+
+ awaitPartitionMapExchange();
+
+ for (int i = 0; i < 100; ++i)
+ assertEquals(1, sql("INSERT INTO TEST_TBL VALUES(" + (i + 1) +
", '" + (i + 1000) + "')").size());
+
+ assertEquals(100, grid(testGrid).cache("TEST_CACHE").size());
+
+ assertEquals(100, sql("SELECT * FROM TEST_TBL").size());
+ }
+ finally {
+ persistence = true;
+
+ afterTestsStopped();
+
+ beforeTestsStarted();
Review Comment:
Looks weird, let's introduce new test class for this test
Also:
- the same problem can be reproduced on H2 engine.
- the same problem exists for persistent (if we clean persistent dir for
testGrid after stopping)
##########
modules/core/src/main/java/org/apache/ignite/internal/processors/cache/ClusterCachesInfo.java:
##########
@@ -1616,37 +1616,35 @@ else if
(!GridFunc.eqNotOrdered(desc.schema().entities(), locQryEntities))
cfg.getNearConfiguration() != null);
}
- updateRegisteredCachesIfNeeded(patchesToApply, cachesToSave,
hasSchemaPatchConflict);
+ if (!hasSchemaPatchConflict)
+ updateRegisteredCaches(patchesToApply, cachesToSave);
}
/**
- * Merging config or resaving it if it needed.
+ * Merging config, resaving it if it needed.
*
* @param patchesToApply Patches which need to apply.
* @param cachesToSave Caches which need to resave.
- * @param hasSchemaPatchConflict {@code true} if we have conflict during
making patch.
- */
- private void updateRegisteredCachesIfNeeded(Map<DynamicCacheDescriptor,
QuerySchemaPatch> patchesToApply,
- Collection<DynamicCacheDescriptor> cachesToSave, boolean
hasSchemaPatchConflict) {
- //Skip merge of config if least one conflict was found.
- if (!hasSchemaPatchConflict) {
- boolean isClusterActive = ctx.state().clusterState().active();
-
- //Merge of config for cluster only for inactive grid.
- if (!isClusterActive && !patchesToApply.isEmpty()) {
- for (Map.Entry<DynamicCacheDescriptor, QuerySchemaPatch> entry
: patchesToApply.entrySet()) {
- if (entry.getKey().applySchemaPatch(entry.getValue()))
- saveCacheConfiguration(entry.getKey());
- }
+ */
+ private void updateRegisteredCaches(
+ Map<DynamicCacheDescriptor, QuerySchemaPatch> patchesToApply,
+ Collection<DynamicCacheDescriptor> cachesToSave
+ ) {
+ // Store config only if cluster is nactive.
Review Comment:
nactive -> active
##########
modules/core/src/main/java/org/apache/ignite/internal/processors/cache/ClusterCachesInfo.java:
##########
@@ -1616,37 +1616,35 @@ else if
(!GridFunc.eqNotOrdered(desc.schema().entities(), locQryEntities))
cfg.getNearConfiguration() != null);
}
- updateRegisteredCachesIfNeeded(patchesToApply, cachesToSave,
hasSchemaPatchConflict);
+ if (!hasSchemaPatchConflict)
+ updateRegisteredCaches(patchesToApply, cachesToSave);
}
/**
- * Merging config or resaving it if it needed.
+ * Merging config, resaving it if it needed.
*
* @param patchesToApply Patches which need to apply.
* @param cachesToSave Caches which need to resave.
- * @param hasSchemaPatchConflict {@code true} if we have conflict during
making patch.
- */
- private void updateRegisteredCachesIfNeeded(Map<DynamicCacheDescriptor,
QuerySchemaPatch> patchesToApply,
- Collection<DynamicCacheDescriptor> cachesToSave, boolean
hasSchemaPatchConflict) {
- //Skip merge of config if least one conflict was found.
- if (!hasSchemaPatchConflict) {
- boolean isClusterActive = ctx.state().clusterState().active();
-
- //Merge of config for cluster only for inactive grid.
- if (!isClusterActive && !patchesToApply.isEmpty()) {
- for (Map.Entry<DynamicCacheDescriptor, QuerySchemaPatch> entry
: patchesToApply.entrySet()) {
- if (entry.getKey().applySchemaPatch(entry.getValue()))
- saveCacheConfiguration(entry.getKey());
- }
+ */
+ private void updateRegisteredCaches(
+ Map<DynamicCacheDescriptor, QuerySchemaPatch> patchesToApply,
+ Collection<DynamicCacheDescriptor> cachesToSave
+ ) {
+ // Store config only if cluster is nactive.
+ boolean isClusterActive = ctx.state().clusterState().active();
- for (DynamicCacheDescriptor descriptor : cachesToSave)
- saveCacheConfiguration(descriptor);
- }
- else if (patchesToApply.isEmpty()) {
- for (DynamicCacheDescriptor descriptor : cachesToSave)
- saveCacheConfiguration(descriptor);
+ //Merge of config for cluster only for inactive grid.
+ if (!patchesToApply.isEmpty()) {
+ for (Map.Entry<DynamicCacheDescriptor, QuerySchemaPatch> entry :
patchesToApply.entrySet()) {
+ if (entry.getKey().applySchemaPatch(entry.getValue()) &&
!isClusterActive)
+ saveCacheConfiguration(entry.getKey());
Review Comment:
Method `saveCacheConfiguration` also called from `processJoiningNode`
without state check. Do we need to add this check to `processJoiningNode` too?
Why do we need state check at all? If it was stored on inactive cluster
state in 2/3 of cases, perhaps, it should be stored in all cases? Does cache
configuration stored after activation? For example, new node has joined the
inactive persistent cluster. Cluster has caches with patches to apply. After
activation cluster works some time and shutted down. Will new query entities be
available on the new node if it will be started as first node?
--
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: notifications-unsubscr...@ignite.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
