anton-vinogradov commented on code in PR #10533:
URL: https://github.com/apache/ignite/pull/10533#discussion_r1108515099
##########
modules/core/src/main/java/org/apache/ignite/internal/processors/cache/CacheGroupContext.java:
##########
@@ -1235,6 +1240,16 @@ public boolean globalWalEnabled() {
return globalWalEnabled;
}
+ /** @return {@code True} if WAL for index operations enabled. */
+ public boolean indexWalEnabled() {
+ return idxWalEnabled && walEnabled();
Review Comment:
`return idxWalEnabled;` seems to be enough at the current code
##########
modules/core/src/main/java/org/apache/ignite/internal/processors/cache/persistence/file/FilePageStoreManager.java:
##########
@@ -437,17 +439,25 @@ private List<CacheConfiguration>
findCacheGroupsWithDisabledWal() {
}
/** {@inheritDoc} */
- @Override public void initialize(int cacheId, int partitions, String
workingDir, PageMetrics pageMetrics)
+ @Override public void initialize(int cacheId, int partitions, String
cacheName, PageMetrics pageMetrics)
throws IgniteCheckedException {
assert storeWorkDir != null;
if (!idxCacheStores.containsKey(cacheId)) {
+ GridCacheContext<?, ?> cctx = this.cctx.cacheContext(cacheId);
+
+ Collection<String> grpCaches = cctx != null
+ ?
cctx.group().caches().stream().map(GridCacheContext::name).collect(Collectors.toSet())
+ : Collections.singleton(cacheName);
Review Comment:
Since you still transfering cacheName, could you encapsulate this to initDir?
BTW, cacheId seems to be enough (no reason to transter cacheName)
##########
modules/core/src/main/java/org/apache/ignite/internal/managers/indexing/IndexesRebuildTask.java:
##########
@@ -121,6 +134,28 @@ public class IndexesRebuildTask {
cctx.kernalContext().query().onFinishRebuildIndexes(cctx);
idxRebuildFuts.remove(cctx.cacheId(), intRebFut);
+
+ if (recreate) {
+ boolean recreateContinues = false;
+
+ for (GridCacheContext<?, ?> cctx0 : cctx.group().caches()) {
+ if (idxRebuildFuts.containsKey(cctx0.cacheId())) {
+ recreateContinues = true;
+
+ break;
+ }
+ }
+
+ if (!recreateContinues) {
+
cctx.group().indexWalEnabled(cctx.group().localWalEnabled());
Review Comment:
Must be cctx.group().indexWalEnabled(true)?
Local wal may be disabled for now, but on enabling you'll still have
indexWal disabled?
##########
modules/core/src/main/java/org/apache/ignite/internal/processors/cache/WalStateManager.java:
##########
@@ -1013,12 +1015,14 @@ private WalStateResult
awaitCheckpoint(CheckpointProgress cpFut, WalStatePropose
* Checks WAL disabled for cache group.
*
* @param grpId Group id.
+ * @param pageId Page id.
* @return {@code True} if WAL disable for group. {@code False} If not.
*/
- public boolean isDisabled(int grpId) {
+ public boolean isDisabled(int grpId, long pageId) {
CacheGroupContext ctx = cctx.cache().cacheGroup(grpId);
- return ctx != null && !ctx.walEnabled();
+ return ctx != null && (!ctx.walEnabled()
+ || (!ctx.indexWalEnabled() && PageIdUtils.partId(pageId) ==
INDEX_PARTITION));
Review Comment:
Relocation of `PageIdUtils.partId(pageId) == INDEX_PARTITION))` to the
`ctx.indexWalEnabled()` should simplify the code.
--
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]
