Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

IGNITE-14383 No need to update metastorage if cache statistics disabled #8924

Open
wants to merge 10 commits into
base: master
Choose a base branch
from
Open

IGNITE-14383 No need to update metastorage if cache statistics disabled #8924

Show file tree
Hide file tree
Changes from 8 commits
Commits
Show all changes
10 commits
Select commit Hold shift + click to select a range
712f677
IGNITE-14383 No need to update metastorage if cache statistics disabled
zstan Mar 24, 2021
6b190b0
fix test
zstan Mar 24, 2021
843844f
test fix
zstan Mar 24, 2021
05de072
fix
zstan Mar 24, 2021
a5dcc85
fix
zstan Mar 24, 2021
55e1d9f
tests ok
zstan Mar 24, 2021
fdff8fd
param
zstan Mar 24, 2021
68d76f8
fix
zstan Mar 25, 2021
db5f102
fix after review
zstan Mar 26, 2021
aba49bb
fix
zstan Mar 26, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
42 changes: 31 additions & 11 deletions ...es/core/src/main/java/org/apache/ignite/internal/processors/cache/GridCacheProcessor.java
View file
Open in desktop
Expand Up @@ -542,8 +542,9 @@ private void cleanup(GridCacheContext cctx) {
/**
* @param grp Cache group.
* @param destroy Group destroy flag.
* @param updateMetrics Remove metrics flag.
zstan marked this conversation as resolved.
Show resolved Hide resolved
*/
private void cleanup(CacheGroupContext grp, boolean destroy) {
private void cleanup(CacheGroupContext grp, boolean destroy, boolean updateMetrics) {
CacheConfiguration cfg = grp.config();

for (Object obj : grp.configuredUserObjects())
Expand All @@ -559,7 +560,7 @@ private void cleanup(CacheGroupContext grp, boolean destroy) {
}
}

if (destroy) {
if (destroy && updateMetrics) {
grp.metrics().remove();

grp.removeIOStatistic();
Expand Down Expand Up @@ -779,8 +780,10 @@ public void stopCaches(boolean cancel) {
stopCache(cache, cancel, false);
}

boolean stat = stoppedCaches.values().stream().anyMatch(c -> c.context().statisticsEnabled());
zstan marked this conversation as resolved.
Show resolved Hide resolved

for (CacheGroupContext grp : cacheGrps.values())
stopCacheGroup(grp.groupId(), false);
stopCacheGroup(grp.groupId(), false, stat);
}

/**
Expand Down Expand Up @@ -2079,7 +2082,7 @@ private void stopCacheSafely(GridCacheContext<?, ?> cctx, boolean clearDbObjects
prepareCacheStop(cctx.name(), false, clearDbObjects);

if (!cctx.group().hasCaches())
stopCacheGroup(cctx.group().groupId(), false);
stopCacheGroup(cctx.group().groupId(), false, cctx.statisticsEnabled());
}
finally {
sharedCtx.database().checkpointReadUnlock();
Expand Down Expand Up @@ -2780,6 +2783,16 @@ private void processCacheStopRequestOnExchangeDone(ExchangeActions exchActions)
Map<Integer, List<ExchangeActions.CacheActionData>> cachesToStop = exchActions.cacheStopRequests().stream()
.collect(groupingBy(action -> action.descriptor().groupId()));

Map<Integer, Boolean> grpMetricsToStop = new HashMap<>();

for (ExchangeActions.CacheActionData actData : exchActions.cacheStopRequests()) {
GridCacheContext<?, ?> cctx = sharedCtx.cacheContext(actData.descriptor().cacheId());

if (cctx != null)
zstan marked this conversation as resolved.
Show resolved Hide resolved
grpMetricsToStop.compute(actData.descriptor().groupId(), (k, v) -> v == null ?
cctx.statisticsEnabled() : v | cctx.statisticsEnabled());
}

try {
doInParallel(
parallelismLvl,
Expand Down Expand Up @@ -2839,8 +2852,11 @@ private void processCacheStopRequestOnExchangeDone(ExchangeActions exchActions)
throw new IgniteException(msg, e);
}

for (IgniteBiTuple<CacheGroupContext, Boolean> grp : grpsToStop)
stopCacheGroup(grp.get1().groupId(), grp.get2());
for (IgniteBiTuple<CacheGroupContext, Boolean> grp : grpsToStop) {
Boolean stat = grpMetricsToStop.get(grp.get1().groupId());
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Are we really need intermediate grpMetricsToStop map?
Let's just calculate statisticsEnabled flag right in place.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yep ... cause upper we removes it context : prepareCacheStop -> sharedCtx.removeCacheContext(ctx);

zstan marked this conversation as resolved.
Show resolved Hide resolved

stopCacheGroup(grp.get1().groupId(), grp.get2(), stat == null ? false : stat);
}

if (!sharedCtx.kernalContext().clientNode())
sharedCtx.database().onCacheGroupsStopped(grpsToStop);
Expand Down Expand Up @@ -2943,24 +2959,26 @@ public void onExchangeDone(
/**
* @param grpId Group ID.
* @param destroy Group destroy flag.
* @param updateMetrics Remove metrics flag.
zstan marked this conversation as resolved.
Show resolved Hide resolved
*/
private void stopCacheGroup(int grpId, boolean destroy) {
private void stopCacheGroup(int grpId, boolean destroy, boolean updateMetrics) {
CacheGroupContext grp = cacheGrps.remove(grpId);

if (grp != null)
stopCacheGroup(grp, destroy);
stopCacheGroup(grp, destroy, updateMetrics);
}

/**
* @param grp Cache group.
* @param destroy Group destroy flag.
* @param updateMetrics Remove metrics flag.
*/
private void stopCacheGroup(CacheGroupContext grp, boolean destroy) {
private void stopCacheGroup(CacheGroupContext grp, boolean destroy, boolean updateMetrics) {
grp.stopGroup();

U.stopLifecycleAware(log, grp.configuredUserObjects());

cleanup(grp, destroy);
cleanup(grp, destroy, updateMetrics);
}

/**
Expand Down Expand Up @@ -3262,6 +3280,8 @@ public void processStatisticsModeChange(CacheStatisticsModeChangeMessage msg) {
private void stopCachesOnClientReconnect(Collection<GridCacheAdapter> stoppedCaches) {
assert ctx.discovery().localNode().isClient();

boolean stat = stoppedCaches.stream().anyMatch(c -> c.context().statisticsEnabled());
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

stat -> removeMetrics

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

done


for (GridCacheAdapter cache : stoppedCaches) {
CacheGroupContext grp = cache.context().group();

Expand All @@ -3271,7 +3291,7 @@ private void stopCachesOnClientReconnect(Collection<GridCacheAdapter> stoppedCac
sharedCtx.affinity().stopCacheOnReconnect(cache.context());

if (!grp.hasCaches()) {
stopCacheGroup(grp, false);
stopCacheGroup(grp, false, stat);

sharedCtx.affinity().stopCacheGroupOnReconnect(grp);
}
Expand Down
2 changes: 2 additions & 0 deletions modules/core/src/test/java/org/apache/ignite/internal/metric/CacheMetricsAddRemoveTest.java
View file
Open in desktop
Expand Up @@ -146,6 +146,8 @@ private void createCache() throws InterruptedException {
private void createCache(@Nullable String dataRegionName, @Nullable String cacheName) throws InterruptedException {
CacheConfiguration ccfg = new CacheConfiguration(DEFAULT_CACHE_NAME);

ccfg.setStatisticsEnabled(true);

if (dataRegionName != null)
ccfg.setDataRegionName(dataRegionName);

Expand Down
16 changes: 10 additions & 6 deletions modules/core/src/test/java/org/apache/ignite/internal/metric/JmxExporterSpiTest.java
View file
Open in desktop
Expand Up @@ -228,18 +228,22 @@ public void testDataRegionJmxMetrics() throws Exception {

/** */
@Test
public void testUnregisterRemovedRegistry() throws Exception {
String n = "cache-for-remove";
public void testUnregisterRemovedRegistry() {
final String cacheName = "cache-for-remove";

IgniteCache c = ignite.createCache(n);
CacheConfiguration<Object, Object> ccfg = new CacheConfiguration(cacheName);

DynamicMBean cacheBean = metricRegistry(ignite.name(), CACHE_METRICS, n);
ccfg.setStatisticsEnabled(true);

IgniteCache c = ignite.createCache(ccfg);

DynamicMBean cacheBean = metricRegistry(ignite.name(), CACHE_METRICS, cacheName);

assertNotNull(cacheBean);

ignite.destroyCache(n);
ignite.destroyCache(c.getName());

assertThrowsWithCause(() -> metricRegistry(ignite.name(), CACHE_METRICS, n), IgniteException.class);
assertThrowsWithCause(() -> metricRegistry(ignite.name(), CACHE_METRICS, cacheName), IgniteException.class);
}

/** */
Expand Down