akalash commented on a change in pull request #8148: URL: https://github.com/apache/ignite/pull/8148#discussion_r473045197
########## File path: modules/core/src/main/java/org/apache/ignite/internal/processors/cache/warmup/WarmUpStrategy.java ########## @@ -0,0 +1,53 @@ +/* + * 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.ignite.internal.processors.cache.warmup; + +import org.apache.ignite.IgniteCheckedException; +import org.apache.ignite.configuration.WarmUpConfiguration; +import org.apache.ignite.internal.GridKernalContext; +import org.apache.ignite.internal.processors.cache.persistence.DataRegion; + +/** + * Interface for warming up. + */ +public interface WarmUpStrategy<T extends WarmUpConfiguration> { + /** + * Returns configuration class for mapping to strategy. + * + * @return Configuration class. + */ + Class<T> configClass(); + + /** + * Warm up. + * + * @param kernalCtx Kernal context. + * @param cfg Warm-up configuration. + * @param region Data region. + * @throws IgniteCheckedException if faild. + */ + void warmUp(GridKernalContext kernalCtx, T cfg, DataRegion region) throws IgniteCheckedException; Review comment: I don't think that the strategy API should contain kernalCtx. kernalCtx is internal part of the ignite but strategy can be implemented in plugins so it is not so internal. kernalCtx binds components too tightly which is the opposite of our target - to make components such independent as possible. coherence | -- | -- ########## File path: modules/core/src/main/java/org/apache/ignite/internal/processors/cache/warmup/NoOpWarmUpConfiguration.java ########## @@ -0,0 +1,31 @@ +/* + * 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.ignite.internal.processors.cache.warmup; Review comment: Again, I don't sure about internal ########## File path: modules/core/src/main/java/org/apache/ignite/internal/IgniteKernal.java ########## @@ -1274,6 +1276,17 @@ public void start( startTimer.finishGlobalStage("Init metastore"); + if (!U.IGNITE_MBEANS_DISABLED) { + WarmUpMXBean warmUpMXBean = new WarmUpMXBeanImpl(ctx); + + mBeansMgr.registerMBean( + "WarmUp", + warmUpMXBean.getClass().getSimpleName(), + warmUpMXBean, + WarmUpMXBean.class + ); + } Review comment: In my opinion, it is better to use IgniteMBeansManager here. You can refactor it such that instead of one method registerAllBeans, it can contain two methods like registerDuringInitPhase and registerAfterNodeStarted ########## File path: modules/core/src/main/java/org/apache/ignite/internal/processors/cache/warmup/LoadAllWarmUpConfiguration.java ########## @@ -0,0 +1,31 @@ +/* + * 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.ignite.internal.processors.cache.warmup; Review comment: I don't think that the internal package is the right place for this class because it is a public configuration. ########## File path: modules/core/src/main/java/org/apache/ignite/configuration/DataRegionConfiguration.java ########## @@ -142,6 +143,9 @@ */ private boolean lazyMemoryAllocation = true; + /** Warm-up configuration. */ + @Nullable private transient WarmUpConfiguration warmUpCfg; Review comment: Why is it transient? ########## File path: modules/core/src/main/java/org/apache/ignite/internal/processors/cache/warmup/WarmUpMXBeanImpl.java ########## @@ -0,0 +1,56 @@ +/* + * 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.ignite.internal.processors.cache.warmup; + +import org.apache.ignite.IgniteCheckedException; +import org.apache.ignite.IgniteException; +import org.apache.ignite.internal.GridKernalContext; +import org.apache.ignite.internal.util.typedef.internal.S; +import org.apache.ignite.mxbean.WarmUpMXBean; + +/** + * {@link WarmUpMXBean} implementation. + */ +public class WarmUpMXBeanImpl implements WarmUpMXBean { + /** Kernal context. */ + private final GridKernalContext kernalCtx; Review comment: Please, avoid context here. This class requires the only cacheProcess so you can use it directly instead of getting it from context. ########## File path: modules/core/src/main/java/org/apache/ignite/internal/processors/cache/GridCacheProcessor.java ########## @@ -5477,6 +5494,96 @@ private void restorePartitionStates( ", partitionsProcessed=" + totalProcessed.get() + ", time=" + (U.currentTimeMillis() - startRestorePart) + "ms]"); } + + /** + * Start warming up sequentially for each persist data region. + * + * @throws IgniteCheckedException If failed. + */ + private void startWarmUp() throws IgniteCheckedException { + boolean start = false; + + try { + // Collecting custom and default data regions. + DataStorageConfiguration dsCfg = ctx.config().getDataStorageConfiguration(); + + List<DataRegionConfiguration> regCfgs = + new ArrayList<>(asList(dsCfg.getDefaultDataRegionConfiguration())); + + if (nonNull(dsCfg.getDataRegionConfigurations())) + regCfgs.addAll(asList(dsCfg.getDataRegionConfigurations())); + + // Warm-up start. + Map<Class<? extends WarmUpConfiguration>, WarmUpStrategy> warmUpStrats = CU.warmUpStrategies(ctx); + + WarmUpConfiguration dfltWarmUpCfg = dsCfg.getDefaultWarmUpConfiguration(); + + for (DataRegionConfiguration regCfg : regCfgs) { + if (stopWarmUp.get()) + return; + + if (!regCfg.isPersistenceEnabled()) + continue; + + WarmUpConfiguration warmUpCfg = nonNull(regCfg.getWarmUpConfiguration()) ? + regCfg.getWarmUpConfiguration() : dfltWarmUpCfg; + + if (isNull(warmUpCfg)) + continue; + + WarmUpStrategy warmUpStrat = (curWarmUpStrat = warmUpStrats.get(warmUpCfg.getClass())); + + DataRegion region = sharedCtx.database().dataRegion(regCfg.getName()); + + if (!stopWarmUp.get()) { Review comment: The current implementation doesn't allow stopping warm-up during the process on the one region which can be a problem. ---------------------------------------------------------------- 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. For queries about this service, please contact Infrastructure at: [email protected]
