Re: [PR] IGNITE-23901 Add operation time performance statistics for putAllConflict, removeAllConflict [ignite-extensions]
NSAmelchev merged PR #299: URL: https://github.com/apache/ignite-extensions/pull/299 -- 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
Re: [PR] IGNITE-23901 Add operation time performance statistics for putAllConflict, removeAllConflict [ignite]
NSAmelchev merged PR #11793: URL: https://github.com/apache/ignite/pull/11793 -- 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
Re: [PR] IGNITE-23901 Add operation time performance statistics for putAllConflict, removeAllConflict [ignite]
sonarqubecloud[bot] commented on PR #11793: URL: https://github.com/apache/ignite/pull/11793#issuecomment-2693989775 ## [](https://sonarcloud.io/dashboard?id=apache_ignite&pullRequest=11793) **Quality Gate passed** Issues  [0 New issues](https://sonarcloud.io/project/issues?id=apache_ignite&pullRequest=11793&issueStatuses=OPEN,CONFIRMED&sinceLeakPeriod=true)  [0 Accepted issues](https://sonarcloud.io/project/issues?id=apache_ignite&pullRequest=11793&issueStatuses=ACCEPTED) Measures  [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_ignite&pullRequest=11793&issueStatuses=OPEN,CONFIRMED&sinceLeakPeriod=true)  [0.0% Coverage on New Code](https://sonarcloud.io/component_measures?id=apache_ignite&pullRequest=11793&metric=new_coverage&view=list)  [0.0% Duplication on New Code](https://sonarcloud.io/component_measures?id=apache_ignite&pullRequest=11793&metric=new_duplicated_lines_density&view=list) [See analysis details on SonarQube Cloud](https://sonarcloud.io/dashboard?id=apache_ignite&pullRequest=11793) -- 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
Re: [PR] IGNITE-23901 Add operation time performance statistics for putAllConflict, removeAllConflict [ignite]
maksaska commented on code in PR #11793:
URL: https://github.com/apache/ignite/pull/11793#discussion_r1958689997
##
modules/core/src/test/java/org/apache/ignite/internal/processors/performancestatistics/PerformanceStatisticsThinClientTest.java:
##
@@ -171,6 +207,68 @@ public void testCacheOperation() throws Exception {
checkCacheOperation(CACHE_GET_AND_REMOVE, cache ->
cache.getAndRemove(5));
}
+/**
+ * Cache {@link TcpClientCache#putAllConflict} operation performed.
+ * @throws Exception If failed.
+ */
+@Test
+public void testCachePutAllConflict() throws Exception {
+checkCacheAllConflictOperations(CACHE_PUT_ALL_CONFLICT, false);
+}
+
+/**
+ * Cache {@link TcpClientCache#removeAllConflict} operation performed.
+ * @throws Exception If failed.
+ */
+@Test
+public void testCacheRemoveAllConflict() throws Exception {
+checkCacheAllConflictOperations(CACHE_REMOVE_ALL_CONFLICT, false);
+}
+
+/**
+ * Cache {@link TcpClientCache#putAllConflictAsync} operation performed.
+ * @throws Exception If failed.
+ */
+@Test
+public void testCachePutAllConflictAsync() throws Exception {
+checkCacheAllConflictOperations(CACHE_PUT_ALL_CONFLICT, true);
+}
+
+/**
+ * Cache {@link TcpClientCache#removeAllConflictAsync} operation performed.
+ * @throws Exception If failed.
+ */
+@Test
+public void testCacheRemoveAllConflictAsync() throws Exception {
+checkCacheAllConflictOperations(CACHE_REMOVE_ALL_CONFLICT, true);
+}
+
+/**
+ * @param opType {@link OperationType} cache operation type.
+ * @param isAsync boolean flag for asynchronous cache operation processing.
+ */
+private void checkCacheAllConflictOperations(OperationType opType, boolean
isAsync) throws Exception {
+GridCacheVersion confl = new GridCacheVersion(1, 0, 1, (byte)2);
+
+checkCacheOperation(opType, cache -> {
+TcpClientCache clientCache =
(TcpClientCache)cache;
+
+try {
+if (opType == CACHE_PUT_ALL_CONFLICT && !isAsync)
+clientCache.putAllConflict(F.asMap(6, new T3<>(1, confl,
CU.EXPIRE_TIME_ETERNAL)));
+else if (opType == CACHE_REMOVE_ALL_CONFLICT && !isAsync)
+clientCache.removeAllConflict(F.asMap(6, confl));
+else if (opType == CACHE_PUT_ALL_CONFLICT)
+clientCache.putAllConflictAsync(F.asMap(7, new T3<>(2,
confl, CU.EXPIRE_TIME_ETERNAL))).get();
+else if (opType == CACHE_REMOVE_ALL_CONFLICT)
+clientCache.removeAllConflictAsync(F.asMap(7,
confl)).get();
+}
+catch (InterruptedException | ExecutionException e) {
Review Comment:
Done!
--
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
Re: [PR] IGNITE-23901 Add operation time performance statistics for putAllConflict, removeAllConflict [ignite]
NSAmelchev commented on code in PR #11793:
URL: https://github.com/apache/ignite/pull/11793#discussion_r1955044141
##
modules/core/src/test/java/org/apache/ignite/internal/processors/performancestatistics/PerformanceStatisticsThinClientTest.java:
##
@@ -171,6 +207,68 @@ public void testCacheOperation() throws Exception {
checkCacheOperation(CACHE_GET_AND_REMOVE, cache ->
cache.getAndRemove(5));
}
+/**
+ * Cache {@link TcpClientCache#putAllConflict} operation performed.
+ * @throws Exception If failed.
+ */
+@Test
+public void testCachePutAllConflict() throws Exception {
+checkCacheAllConflictOperations(CACHE_PUT_ALL_CONFLICT, false);
+}
+
+/**
+ * Cache {@link TcpClientCache#removeAllConflict} operation performed.
+ * @throws Exception If failed.
+ */
+@Test
+public void testCacheRemoveAllConflict() throws Exception {
+checkCacheAllConflictOperations(CACHE_REMOVE_ALL_CONFLICT, false);
+}
+
+/**
+ * Cache {@link TcpClientCache#putAllConflictAsync} operation performed.
+ * @throws Exception If failed.
+ */
+@Test
+public void testCachePutAllConflictAsync() throws Exception {
+checkCacheAllConflictOperations(CACHE_PUT_ALL_CONFLICT, true);
+}
+
+/**
+ * Cache {@link TcpClientCache#removeAllConflictAsync} operation performed.
+ * @throws Exception If failed.
+ */
+@Test
+public void testCacheRemoveAllConflictAsync() throws Exception {
+checkCacheAllConflictOperations(CACHE_REMOVE_ALL_CONFLICT, true);
+}
+
+/**
+ * @param opType {@link OperationType} cache operation type.
+ * @param isAsync boolean flag for asynchronous cache operation processing.
+ */
+private void checkCacheAllConflictOperations(OperationType opType, boolean
isAsync) throws Exception {
+GridCacheVersion confl = new GridCacheVersion(1, 0, 1, (byte)2);
+
+checkCacheOperation(opType, cache -> {
+TcpClientCache clientCache =
(TcpClientCache)cache;
+
+try {
+if (opType == CACHE_PUT_ALL_CONFLICT && !isAsync)
+clientCache.putAllConflict(F.asMap(6, new T3<>(1, confl,
CU.EXPIRE_TIME_ETERNAL)));
+else if (opType == CACHE_REMOVE_ALL_CONFLICT && !isAsync)
+clientCache.removeAllConflict(F.asMap(6, confl));
+else if (opType == CACHE_PUT_ALL_CONFLICT)
+clientCache.putAllConflictAsync(F.asMap(7, new T3<>(2,
confl, CU.EXPIRE_TIME_ETERNAL))).get();
+else if (opType == CACHE_REMOVE_ALL_CONFLICT)
+clientCache.removeAllConflictAsync(F.asMap(7,
confl)).get();
+}
+catch (InterruptedException | ExecutionException e) {
Review Comment:
Lets use `ConsumerX` instead of catching.
--
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
Re: [PR] IGNITE-23901 Add operation time performance statistics for putAllConflict, removeAllConflict [ignite]
maksaska commented on code in PR #11793:
URL: https://github.com/apache/ignite/pull/11793#discussion_r1948066246
##
modules/core/src/test/java/org/apache/ignite/internal/processors/performancestatistics/PerformanceStatisticsThinClientTest.java:
##
@@ -96,7 +119,19 @@ public class PerformanceStatisticsThinClientTest extends
AbstractPerformanceStat
@Override protected void afterTestsStopped() throws Exception {
super.afterTestsStopped();
-thinClient.close();
+stopAllGrids();
+}
+
+/** {@inheritDoc} */
+@Override protected void beforeTest() throws Exception {
+thinClient.getOrCreateCache(new ClientCacheConfiguration()
Review Comment:
Fixed it: Now createCache() is used to start the cache with updated
configuration. After each test the cache is destroyed
##
modules/core/src/test/java/org/apache/ignite/internal/processors/performancestatistics/PerformanceStatisticsThinClientTest.java:
##
@@ -66,12 +81,20 @@ public class PerformanceStatisticsThinClientTest extends
AbstractPerformanceStat
/** Thin client. */
private static IgniteClient thinClient;
+/** */
+@Parameterized.Parameter
+public CacheAtomicityMode atomicityMode;
+
+/** */
+@Parameterized.Parameters(name = "atomicity={0}")
Review Comment:
fixed
--
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
Re: [PR] IGNITE-23901 Add operation time performance statistics for putAllConflict, removeAllConflict [ignite]
maksaska commented on code in PR #11793:
URL: https://github.com/apache/ignite/pull/11793#discussion_r1948065981
##
modules/core/src/test/java/org/apache/ignite/internal/processors/performancestatistics/PerformanceStatisticsThinClientTest.java:
##
@@ -96,7 +119,19 @@ public class PerformanceStatisticsThinClientTest extends
AbstractPerformanceStat
@Override protected void afterTestsStopped() throws Exception {
super.afterTestsStopped();
-thinClient.close();
+stopAllGrids();
Review Comment:
Added closure for a thin client
--
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
Re: [PR] IGNITE-23901 Add operation time performance statistics for putAllConflict, removeAllConflict [ignite]
oleg-vlsk commented on code in PR #11793:
URL: https://github.com/apache/ignite/pull/11793#discussion_r1947484280
##
modules/core/src/test/java/org/apache/ignite/internal/processors/performancestatistics/PerformanceStatisticsThinClientTest.java:
##
@@ -96,7 +119,19 @@ public class PerformanceStatisticsThinClientTest extends
AbstractPerformanceStat
@Override protected void afterTestsStopped() throws Exception {
super.afterTestsStopped();
-thinClient.close();
+stopAllGrids();
Review Comment:
The `stopAllGrids` method doesn't stop thin clients.
##
modules/core/src/test/java/org/apache/ignite/internal/processors/performancestatistics/PerformanceStatisticsThinClientTest.java:
##
@@ -66,12 +81,20 @@ public class PerformanceStatisticsThinClientTest extends
AbstractPerformanceStat
/** Thin client. */
private static IgniteClient thinClient;
+/** */
+@Parameterized.Parameter
+public CacheAtomicityMode atomicityMode;
+
+/** */
+@Parameterized.Parameters(name = "atomicity={0}")
Review Comment:
`atomicityMode` to be consistent?
##
modules/core/src/test/java/org/apache/ignite/internal/processors/performancestatistics/PerformanceStatisticsThinClientTest.java:
##
@@ -96,7 +119,19 @@ public class PerformanceStatisticsThinClientTest extends
AbstractPerformanceStat
@Override protected void afterTestsStopped() throws Exception {
super.afterTestsStopped();
-thinClient.close();
+stopAllGrids();
+}
+
+/** {@inheritDoc} */
+@Override protected void beforeTest() throws Exception {
+thinClient.getOrCreateCache(new ClientCacheConfiguration()
Review Comment:
When the `getOrCreateCache` method is called on an existing cache, it
ignores the configuration passed as a parameter (as noted in the Javadoc). As a
result, all your tests are run with the same atomicity mode twice (whichever
mode was used in the initial configuration).
Starting new grids before each test is perfectly reasonable since it doesn’t
take up much time. You can opt for this approach if necessary.
--
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
Re: [PR] IGNITE-23901 Add operation time performance statistics for putAllConflict, removeAllConflict [ignite]
maksaska commented on code in PR #11793:
URL: https://github.com/apache/ignite/pull/11793#discussion_r1928405563
##
modules/core/src/test/java/org/apache/ignite/internal/processors/performancestatistics/PerformanceStatisticsThinClientTest.java:
##
@@ -171,6 +178,87 @@ public void testCacheOperation() throws Exception {
checkCacheOperation(CACHE_GET_AND_REMOVE, cache ->
cache.getAndRemove(5));
}
+/**
+ * Cache {@link TcpClientCache#putAllConflict} operation performed.
+ * @throws Exception If failed.
+ */
+@Test
+public void testCachePutAllConflict() throws Exception {
Review Comment:
I used the "cache" in naming to match the OperationType. e.g.
CACHE_PUT_ALL_CONFLICT
I agree, it can result in less code should we remove it, but I see that it's
used throughout the test class. I would prefer to follow that. WDYT?
--
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
Re: [PR] IGNITE-23901 Add operation time performance statistics for putAllConflict, removeAllConflict [ignite]
oleg-vlsk commented on code in PR #11793:
URL: https://github.com/apache/ignite/pull/11793#discussion_r1926028218
##
modules/core/src/test/java/org/apache/ignite/internal/processors/performancestatistics/PerformanceStatisticsThinClientTest.java:
##
@@ -171,6 +178,87 @@ public void testCacheOperation() throws Exception {
checkCacheOperation(CACHE_GET_AND_REMOVE, cache ->
cache.getAndRemove(5));
}
+/**
+ * Cache {@link TcpClientCache#putAllConflict} operation performed.
+ * @throws Exception If failed.
+ */
+@Test
+public void testCachePutAllConflict() throws Exception {
+checkCacheAllConflictOperations(CACHE_PUT_ALL_CONFLICT, false);
+}
+
+/**
+ * Cache {@link TcpClientCache#removeAllConflict} operation performed.
+ * @throws Exception If failed.
+ */
+@Test
+public void testCacheRemoveAllConflict() throws Exception {
+checkCacheAllConflictOperations(CACHE_REMOVE_ALL_CONFLICT, false);
+}
+
+/**
+ * Cache {@link TcpClientCache#putAllConflictAsync} operation performed.
+ * @throws Exception If failed.
+ */
+@Test
+public void testCachePutAllConflictAsync() throws Exception {
+checkCacheAllConflictOperations(CACHE_PUT_ALL_CONFLICT, true);
+}
+
+/**
+ * Cache {@link TcpClientCache#removeAllConflictAsync} operation performed.
+ * @throws Exception If failed.
+ */
+@Test
+public void testCacheRemoveAllConflictAsync() throws Exception {
+checkCacheAllConflictOperations(CACHE_REMOVE_ALL_CONFLICT, true);
+}
+
+/**
+ * @param opType {@link OperationType} cache operation type.
+ * @param isAsync boolean flag for asynchronous cache operation processing.
+ */
+private void checkCacheAllConflictOperations(OperationType opType, boolean
isAsync) throws Exception {
+checkCacheOperation(opType, cache -> {
+try {
+if (opType == CACHE_PUT_ALL_CONFLICT && !isAsync)
+((TcpClientCache)cache).putAllConflict(getPutAllConflictMap(6, 1));
+else if (opType == CACHE_REMOVE_ALL_CONFLICT && !isAsync)
+((TcpClientCache)cache).removeAllConflict(getRemoveAllConflictMap(6));
+else if (opType == CACHE_PUT_ALL_CONFLICT)
+((TcpClientCache)cache).putAllConflictAsync(getPutAllConflictMap(7, 2)).get();
+else if (opType == CACHE_REMOVE_ALL_CONFLICT)
+((TcpClientCache)cache).removeAllConflictAsync(getRemoveAllConflictMap(7)).get();
+}
+catch (InterruptedException | ExecutionException e) {
+throw new RuntimeException(e);
+}
+});
+}
+
+/**
+ * @param key Key
+ * @param val Value
+ * @return {@link Map} with data to send with {@link
TcpClientCache#putAllConflict(Map)} or
+ * {@link TcpClientCache#putAllConflictAsync(Map)} cache operations.
+ */
+private Map> getPutAllConflictMap(int
key, int val) {
+GridCacheVersion confl = new GridCacheVersion(1, 0, 1, (byte)2);
Review Comment:
Code duplication here and in `getRemoveAllConflictMap`. As an option,
`GridCacheVersion confl` could be decalred as a class variable and then
initialized in `checkCacheAllConflictOperations`.
##
modules/core/src/main/java/org/apache/ignite/internal/processors/cache/GridCacheAdapter.java:
##
@@ -2838,9 +2846,10 @@ protected IgniteInternalFuture
removeAsync0(final K key, @Nullable fina
if (F.isEmpty(drMap))
return;
-boolean statsEnabled = ctx.statisticsEnabled();
+final boolean statsEnabled = ctx.statisticsEnabled();
+final boolean performanceStatsEnabled =
ctx.kernalContext().performanceStatistics().enabled();
Review Comment:
Not shortened like other occurences of this variable.
##
modules/core/src/test/java/org/apache/ignite/internal/processors/performancestatistics/PerformanceStatisticsThinClientTest.java:
##
@@ -171,6 +178,87 @@ public void testCacheOperation() throws Exception {
checkCacheOperation(CACHE_GET_AND_REMOVE, cache ->
cache.getAndRemove(5));
}
+/**
+ * Cache {@link TcpClientCache#putAllConflict} operation performed.
+ * @throws Exception If failed.
+ */
+@Test
+public void testCachePutAllConflict() throws Exception {
Review Comment:
I would consider removing the "Cache" substring from the names of the test
methods and `checkCacheAllConflictOperations`, as it doesn't really provide any
additional useful information. This will result in less code and improved
readability.
--
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 Infrastruct
Re: [PR] IGNITE-23901 Add operation time performance statistics for putAllConflict, removeAllConflict [ignite]
maksaska commented on code in PR #11793:
URL: https://github.com/apache/ignite/pull/11793#discussion_r1922545887
##
modules/core/src/main/java/org/apache/ignite/internal/processors/cache/GridCacheAdapter.java:
##
@@ -1969,8 +1969,9 @@ protected boolean put0(final K key, final V val, final
CacheEntryPredicate filte
return;
final boolean statsEnabled = ctx.statisticsEnabled();
+final boolean performanceStatsEnabled =
ctx.kernalContext().performanceStatistics().enabled();
Review Comment:
Yep, it sounds reasonable
##
modules/core/src/test/java/org/apache/ignite/internal/processors/performancestatistics/PerformanceStatisticsThinClientTest.java:
##
@@ -171,6 +178,75 @@ public void testCacheOperation() throws Exception {
checkCacheOperation(CACHE_GET_AND_REMOVE, cache ->
cache.getAndRemove(5));
}
+/**
+ * Cache {@link TcpClientCache#putAllConflict} operation performed.
+ * @throws Exception If failed.
+ */
+@Test
+public void testCachePutAllConflict() throws Exception {
+checkCacheAllConflictOperations(CACHE_PUT_ALL_CONFLICT, false);
+}
+
+/**
+ * Cache {@link TcpClientCache#removeAllConflict} operation performed.
+ * @throws Exception If failed.
+ */
+@Test
+public void testCacheRemoveAllConflict() throws Exception {
+checkCacheAllConflictOperations(CACHE_REMOVE_ALL_CONFLICT, false);
+}
+
+/**
+ * Cache {@link TcpClientCache#putAllConflictAsync} operation performed.
+ * @throws Exception If failed.
+ */
+@Test
+public void testCachePutAllConflictAsync() throws Exception {
+checkCacheAllConflictOperations(CACHE_PUT_ALL_CONFLICT, true);
+}
+
+/**
+ * Cache {@link TcpClientCache#removeAllConflictAsync} operation performed.
+ * @throws Exception If failed.
+ */
+@Test
+public void testCacheRemoveAllConflictAsync() throws Exception {
+checkCacheAllConflictOperations(CACHE_REMOVE_ALL_CONFLICT, true);
+}
+
+/**
+ * @param isAsync boolean flag for asynchronous cache operation processing.
+ */
+private void checkCacheAllConflictOperations(OperationType opType, boolean
isAsync) throws Exception {
+int key = 6;
+int val = 1;
+
+GridCacheVersion confl = new GridCacheVersion(1, 0, 1, (byte)2);
+
+Map> putMap = F.asMap(key, new
T3<>(val, confl, CU.EXPIRE_TIME_ETERNAL));
+Map rmvMap = F.asMap(key, confl);
+
+if (!isAsync) {
+if (opType.equals(CACHE_PUT_ALL_CONFLICT))
Review Comment:
Done
--
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
Re: [PR] IGNITE-23901 Add operation time performance statistics for putAllConflict, removeAllConflict [ignite]
maksaska commented on code in PR #11793:
URL: https://github.com/apache/ignite/pull/11793#discussion_r1922547785
##
modules/core/src/test/java/org/apache/ignite/internal/processors/performancestatistics/PerformanceStatisticsThinClientTest.java:
##
@@ -171,6 +178,75 @@ public void testCacheOperation() throws Exception {
checkCacheOperation(CACHE_GET_AND_REMOVE, cache ->
cache.getAndRemove(5));
}
+/**
+ * Cache {@link TcpClientCache#putAllConflict} operation performed.
+ * @throws Exception If failed.
+ */
+@Test
+public void testCachePutAllConflict() throws Exception {
+checkCacheAllConflictOperations(CACHE_PUT_ALL_CONFLICT, false);
+}
+
+/**
+ * Cache {@link TcpClientCache#removeAllConflict} operation performed.
+ * @throws Exception If failed.
+ */
+@Test
+public void testCacheRemoveAllConflict() throws Exception {
+checkCacheAllConflictOperations(CACHE_REMOVE_ALL_CONFLICT, false);
+}
+
+/**
+ * Cache {@link TcpClientCache#putAllConflictAsync} operation performed.
+ * @throws Exception If failed.
+ */
+@Test
+public void testCachePutAllConflictAsync() throws Exception {
+checkCacheAllConflictOperations(CACHE_PUT_ALL_CONFLICT, true);
+}
+
+/**
+ * Cache {@link TcpClientCache#removeAllConflictAsync} operation performed.
+ * @throws Exception If failed.
+ */
+@Test
+public void testCacheRemoveAllConflictAsync() throws Exception {
+checkCacheAllConflictOperations(CACHE_REMOVE_ALL_CONFLICT, true);
+}
+
+/**
+ * @param isAsync boolean flag for asynchronous cache operation processing.
+ */
+private void checkCacheAllConflictOperations(OperationType opType, boolean
isAsync) throws Exception {
+int key = 6;
+int val = 1;
+
+GridCacheVersion confl = new GridCacheVersion(1, 0, 1, (byte)2);
+
+Map> putMap = F.asMap(key, new
T3<>(val, confl, CU.EXPIRE_TIME_ETERNAL));
+Map rmvMap = F.asMap(key, confl);
+
+if (!isAsync) {
+if (opType.equals(CACHE_PUT_ALL_CONFLICT))
+checkCacheOperation(opType, cache -> ((TcpClientCache)cache).putAllConflict(putMap));
Review Comment:
Utilized closure for all operations async and !async. This way it's more
concise
--
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
Re: [PR] IGNITE-23901 Add operation time performance statistics for putAllConflict, removeAllConflict [ignite]
maksaska commented on code in PR #11793:
URL: https://github.com/apache/ignite/pull/11793#discussion_r1922546435
##
modules/core/src/test/java/org/apache/ignite/internal/processors/performancestatistics/PerformanceStatisticsThinClientTest.java:
##
@@ -171,6 +178,75 @@ public void testCacheOperation() throws Exception {
checkCacheOperation(CACHE_GET_AND_REMOVE, cache ->
cache.getAndRemove(5));
}
+/**
+ * Cache {@link TcpClientCache#putAllConflict} operation performed.
+ * @throws Exception If failed.
+ */
+@Test
+public void testCachePutAllConflict() throws Exception {
+checkCacheAllConflictOperations(CACHE_PUT_ALL_CONFLICT, false);
+}
+
+/**
+ * Cache {@link TcpClientCache#removeAllConflict} operation performed.
+ * @throws Exception If failed.
+ */
+@Test
+public void testCacheRemoveAllConflict() throws Exception {
+checkCacheAllConflictOperations(CACHE_REMOVE_ALL_CONFLICT, false);
+}
+
+/**
+ * Cache {@link TcpClientCache#putAllConflictAsync} operation performed.
+ * @throws Exception If failed.
+ */
+@Test
+public void testCachePutAllConflictAsync() throws Exception {
+checkCacheAllConflictOperations(CACHE_PUT_ALL_CONFLICT, true);
+}
+
+/**
+ * Cache {@link TcpClientCache#removeAllConflictAsync} operation performed.
+ * @throws Exception If failed.
+ */
+@Test
+public void testCacheRemoveAllConflictAsync() throws Exception {
+checkCacheAllConflictOperations(CACHE_REMOVE_ALL_CONFLICT, true);
+}
+
+/**
+ * @param isAsync boolean flag for asynchronous cache operation processing.
+ */
+private void checkCacheAllConflictOperations(OperationType opType, boolean
isAsync) throws Exception {
+int key = 6;
+int val = 1;
+
+GridCacheVersion confl = new GridCacheVersion(1, 0, 1, (byte)2);
+
+Map> putMap = F.asMap(key, new
T3<>(val, confl, CU.EXPIRE_TIME_ETERNAL));
Review Comment:
Created separate methods for maps
--
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
Re: [PR] IGNITE-23901 Add operation time performance statistics for putAllConflict, removeAllConflict [ignite]
maksaska commented on code in PR #11793:
URL: https://github.com/apache/ignite/pull/11793#discussion_r1922545542
##
modules/core/src/test/java/org/apache/ignite/internal/processors/performancestatistics/PerformanceStatisticsThinClientTest.java:
##
@@ -171,6 +178,75 @@ public void testCacheOperation() throws Exception {
checkCacheOperation(CACHE_GET_AND_REMOVE, cache ->
cache.getAndRemove(5));
}
+/**
+ * Cache {@link TcpClientCache#putAllConflict} operation performed.
+ * @throws Exception If failed.
+ */
+@Test
+public void testCachePutAllConflict() throws Exception {
+checkCacheAllConflictOperations(CACHE_PUT_ALL_CONFLICT, false);
+}
+
+/**
+ * Cache {@link TcpClientCache#removeAllConflict} operation performed.
+ * @throws Exception If failed.
+ */
+@Test
+public void testCacheRemoveAllConflict() throws Exception {
+checkCacheAllConflictOperations(CACHE_REMOVE_ALL_CONFLICT, false);
+}
+
+/**
+ * Cache {@link TcpClientCache#putAllConflictAsync} operation performed.
+ * @throws Exception If failed.
+ */
+@Test
+public void testCachePutAllConflictAsync() throws Exception {
+checkCacheAllConflictOperations(CACHE_PUT_ALL_CONFLICT, true);
+}
+
+/**
+ * Cache {@link TcpClientCache#removeAllConflictAsync} operation performed.
+ * @throws Exception If failed.
+ */
+@Test
+public void testCacheRemoveAllConflictAsync() throws Exception {
+checkCacheAllConflictOperations(CACHE_REMOVE_ALL_CONFLICT, true);
+}
+
+/**
+ * @param isAsync boolean flag for asynchronous cache operation processing.
Review Comment:
Fixed it
--
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
Re: [PR] IGNITE-23901 Add operation time performance statistics for putAllConflict, removeAllConflict [ignite]
maksaska commented on code in PR #11793:
URL: https://github.com/apache/ignite/pull/11793#discussion_r1918590889
##
modules/core/src/test/java/org/apache/ignite/internal/processors/performancestatistics/PerformanceStatisticsThinClientTest.java:
##
@@ -171,6 +178,62 @@ public void testCacheOperation() throws Exception {
checkCacheOperation(CACHE_GET_AND_REMOVE, cache ->
cache.getAndRemove(5));
}
+/**
+ * Cache {@link TcpClientCache#putAllConflict} and {@link
TcpClientCache#removeAllConflict} operations performed.
+ * @throws Exception If failed.
+ */
+@Test
+public void testCacheAllConflictOperations() throws Exception {
+checkCacheAllConflictOperations(false);
+}
+
+/**
+ * Cache {@link TcpClientCache#putAllConflictAsync} and {@link
TcpClientCache#removeAllConflictAsync} operations performed.
+ * @throws Exception If failed.
+ */
+@Test
+public void testCacheAllConflictOperationsAsync() throws Exception {
+checkCacheAllConflictOperations(true);
+}
+
+/**
+ * @param async boolean flag for asynchronous cache operation processing.
+ */
+private void checkCacheAllConflictOperations(boolean async) throws
Exception {
+int key = 6;
+int val = 1;
+
+GridCacheVersion confl = new GridCacheVersion(1, 0, 1, (byte)2);
+
+Map> putMap = F.asMap(key, new
T3<>(val, confl, CU.EXPIRE_TIME_ETERNAL));
+Map rmvMap = F.asMap(key, confl);
+
+if (async) {
+checkCacheOperation(CACHE_PUT_ALL_CONFLICT, cache ->
((TcpClientCache)cache).putAllConflict(putMap));
+
+checkCacheOperation(CACHE_REMOVE_ALL_CONFLICT, cache ->
((TcpClientCache)cache).removeAllConflict(rmvMap));
+}
+else {
+checkCacheOperation(CACHE_PUT_ALL_CONFLICT, cache -> {
Review Comment:
Good point
##
modules/core/src/test/java/org/apache/ignite/internal/processors/performancestatistics/PerformanceStatisticsThinClientTest.java:
##
@@ -171,6 +178,62 @@ public void testCacheOperation() throws Exception {
checkCacheOperation(CACHE_GET_AND_REMOVE, cache ->
cache.getAndRemove(5));
}
+/**
+ * Cache {@link TcpClientCache#putAllConflict} and {@link
TcpClientCache#removeAllConflict} operations performed.
+ * @throws Exception If failed.
+ */
+@Test
+public void testCacheAllConflictOperations() throws Exception {
+checkCacheAllConflictOperations(false);
+}
+
+/**
+ * Cache {@link TcpClientCache#putAllConflictAsync} and {@link
TcpClientCache#removeAllConflictAsync} operations performed.
+ * @throws Exception If failed.
+ */
+@Test
+public void testCacheAllConflictOperationsAsync() throws Exception {
+checkCacheAllConflictOperations(true);
+}
+
+/**
+ * @param async boolean flag for asynchronous cache operation processing.
+ */
+private void checkCacheAllConflictOperations(boolean async) throws
Exception {
Review Comment:
Done
--
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
Re: [PR] IGNITE-23901 Add operation time performance statistics for putAllConflict, removeAllConflict [ignite]
maksaska commented on code in PR #11793:
URL: https://github.com/apache/ignite/pull/11793#discussion_r1918590440
##
modules/core/src/test/java/org/apache/ignite/internal/processors/performancestatistics/PerformanceStatisticsThinClientTest.java:
##
@@ -171,6 +178,62 @@ public void testCacheOperation() throws Exception {
checkCacheOperation(CACHE_GET_AND_REMOVE, cache ->
cache.getAndRemove(5));
}
+/**
+ * Cache {@link TcpClientCache#putAllConflict} and {@link
TcpClientCache#removeAllConflict} operations performed.
+ * @throws Exception If failed.
+ */
+@Test
+public void testCacheAllConflictOperations() throws Exception {
+checkCacheAllConflictOperations(false);
Review Comment:
Yeah. I separeted the methods
##
modules/core/src/test/java/org/apache/ignite/internal/processors/performancestatistics/PerformanceStatisticsThinClientTest.java:
##
@@ -171,6 +178,62 @@ public void testCacheOperation() throws Exception {
checkCacheOperation(CACHE_GET_AND_REMOVE, cache ->
cache.getAndRemove(5));
}
+/**
+ * Cache {@link TcpClientCache#putAllConflict} and {@link
TcpClientCache#removeAllConflict} operations performed.
+ * @throws Exception If failed.
+ */
+@Test
+public void testCacheAllConflictOperations() throws Exception {
+checkCacheAllConflictOperations(false);
+}
+
+/**
+ * Cache {@link TcpClientCache#putAllConflictAsync} and {@link
TcpClientCache#removeAllConflictAsync} operations performed.
+ * @throws Exception If failed.
+ */
+@Test
+public void testCacheAllConflictOperationsAsync() throws Exception {
+checkCacheAllConflictOperations(true);
+}
+
+/**
+ * @param async boolean flag for asynchronous cache operation processing.
+ */
+private void checkCacheAllConflictOperations(boolean async) throws
Exception {
+int key = 6;
+int val = 1;
+
+GridCacheVersion confl = new GridCacheVersion(1, 0, 1, (byte)2);
+
+Map> putMap = F.asMap(key, new
T3<>(val, confl, CU.EXPIRE_TIME_ETERNAL));
+Map rmvMap = F.asMap(key, confl);
+
+if (async) {
Review Comment:
Sure
--
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
Re: [PR] IGNITE-23901 Add operation time performance statistics for putAllConflict, removeAllConflict [ignite]
oleg-vlsk commented on code in PR #11793:
URL: https://github.com/apache/ignite/pull/11793#discussion_r1915968446
##
modules/core/src/test/java/org/apache/ignite/internal/processors/performancestatistics/PerformanceStatisticsThinClientTest.java:
##
@@ -171,6 +178,62 @@ public void testCacheOperation() throws Exception {
checkCacheOperation(CACHE_GET_AND_REMOVE, cache ->
cache.getAndRemove(5));
}
+/**
+ * Cache {@link TcpClientCache#putAllConflict} and {@link
TcpClientCache#removeAllConflict} operations performed.
+ * @throws Exception If failed.
+ */
+@Test
+public void testCacheAllConflictOperations() throws Exception {
+checkCacheAllConflictOperations(false);
+}
+
+/**
+ * Cache {@link TcpClientCache#putAllConflictAsync} and {@link
TcpClientCache#removeAllConflictAsync} operations performed.
+ * @throws Exception If failed.
+ */
+@Test
+public void testCacheAllConflictOperationsAsync() throws Exception {
+checkCacheAllConflictOperations(true);
+}
+
+/**
+ * @param async boolean flag for asynchronous cache operation processing.
+ */
+private void checkCacheAllConflictOperations(boolean async) throws
Exception {
Review Comment:
boolean isAsync
--
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
Re: [PR] IGNITE-23901 Add operation time performance statistics for putAllConflict, removeAllConflict [ignite]
oleg-vlsk commented on code in PR #11793:
URL: https://github.com/apache/ignite/pull/11793#discussion_r1915934895
##
modules/core/src/test/java/org/apache/ignite/internal/processors/performancestatistics/PerformanceStatisticsThinClientTest.java:
##
@@ -171,6 +178,62 @@ public void testCacheOperation() throws Exception {
checkCacheOperation(CACHE_GET_AND_REMOVE, cache ->
cache.getAndRemove(5));
}
+/**
+ * Cache {@link TcpClientCache#putAllConflict} and {@link
TcpClientCache#removeAllConflict} operations performed.
+ * @throws Exception If failed.
+ */
+@Test
+public void testCacheAllConflictOperations() throws Exception {
+checkCacheAllConflictOperations(false);
Review Comment:
Looks much better now, but I believe that moving each individual check into
its own test method would make it even more convenient. Please consider adding
the operation type argument to `checkCacheAllConflictOperations` so that it can
be used for all four test methods.
--
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
Re: [PR] IGNITE-23901 Add operation time performance statistics for putAllConflict, removeAllConflict [ignite]
oleg-vlsk commented on code in PR #11793:
URL: https://github.com/apache/ignite/pull/11793#discussion_r1915947859
##
modules/core/src/test/java/org/apache/ignite/internal/processors/performancestatistics/PerformanceStatisticsThinClientTest.java:
##
@@ -171,6 +178,62 @@ public void testCacheOperation() throws Exception {
checkCacheOperation(CACHE_GET_AND_REMOVE, cache ->
cache.getAndRemove(5));
}
+/**
+ * Cache {@link TcpClientCache#putAllConflict} and {@link
TcpClientCache#removeAllConflict} operations performed.
+ * @throws Exception If failed.
+ */
+@Test
+public void testCacheAllConflictOperations() throws Exception {
+checkCacheAllConflictOperations(false);
+}
+
+/**
+ * Cache {@link TcpClientCache#putAllConflictAsync} and {@link
TcpClientCache#removeAllConflictAsync} operations performed.
+ * @throws Exception If failed.
+ */
+@Test
+public void testCacheAllConflictOperationsAsync() throws Exception {
+checkCacheAllConflictOperations(true);
+}
+
+/**
+ * @param async boolean flag for asynchronous cache operation processing.
+ */
+private void checkCacheAllConflictOperations(boolean async) throws
Exception {
+int key = 6;
+int val = 1;
+
+GridCacheVersion confl = new GridCacheVersion(1, 0, 1, (byte)2);
+
+Map> putMap = F.asMap(key, new
T3<>(val, confl, CU.EXPIRE_TIME_ETERNAL));
+Map rmvMap = F.asMap(key, confl);
+
+if (async) {
+checkCacheOperation(CACHE_PUT_ALL_CONFLICT, cache ->
((TcpClientCache)cache).putAllConflict(putMap));
+
+checkCacheOperation(CACHE_REMOVE_ALL_CONFLICT, cache ->
((TcpClientCache)cache).removeAllConflict(rmvMap));
+}
+else {
+checkCacheOperation(CACHE_PUT_ALL_CONFLICT, cache -> {
Review Comment:
Maybe somethig like this to avoid `try-catch` code duplication?
```
checkCacheOperation(type, cache -> {
try {
if (type == CACHE_PUT_ALL_CONFLICT)
((TcpClientCache)cache).putAllConflictAsync(putMap).get();
else if (type == CACHE_REMOVE_ALL_CONFLICT)
((TcpClientCache)cache).removeAllConflictAsync(rmvMap).get();
}
catch (InterruptedException | ExecutionException e) {
throw new RuntimeException(e);
}
});
```
--
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
Re: [PR] IGNITE-23901 Add operation time performance statistics for putAllConflict, removeAllConflict [ignite]
oleg-vlsk commented on code in PR #11793:
URL: https://github.com/apache/ignite/pull/11793#discussion_r1915934895
##
modules/core/src/test/java/org/apache/ignite/internal/processors/performancestatistics/PerformanceStatisticsThinClientTest.java:
##
@@ -171,6 +178,62 @@ public void testCacheOperation() throws Exception {
checkCacheOperation(CACHE_GET_AND_REMOVE, cache ->
cache.getAndRemove(5));
}
+/**
+ * Cache {@link TcpClientCache#putAllConflict} and {@link
TcpClientCache#removeAllConflict} operations performed.
+ * @throws Exception If failed.
+ */
+@Test
+public void testCacheAllConflictOperations() throws Exception {
+checkCacheAllConflictOperations(false);
Review Comment:
Looks much better now, but I believe that moving each individual check into
its own test method would make it even more convenient. Please consider adding
the operation type argiment to `checkCacheAllConflictOperations` so that it can
be used for all four test methods.
--
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
Re: [PR] IGNITE-23901 Add operation time performance statistics for putAllConflict, removeAllConflict [ignite]
oleg-vlsk commented on code in PR #11793:
URL: https://github.com/apache/ignite/pull/11793#discussion_r1915943700
##
modules/core/src/test/java/org/apache/ignite/internal/processors/performancestatistics/PerformanceStatisticsThinClientTest.java:
##
@@ -171,6 +178,62 @@ public void testCacheOperation() throws Exception {
checkCacheOperation(CACHE_GET_AND_REMOVE, cache ->
cache.getAndRemove(5));
}
+/**
+ * Cache {@link TcpClientCache#putAllConflict} and {@link
TcpClientCache#removeAllConflict} operations performed.
+ * @throws Exception If failed.
+ */
+@Test
+public void testCacheAllConflictOperations() throws Exception {
+checkCacheAllConflictOperations(false);
+}
+
+/**
+ * Cache {@link TcpClientCache#putAllConflictAsync} and {@link
TcpClientCache#removeAllConflictAsync} operations performed.
+ * @throws Exception If failed.
+ */
+@Test
+public void testCacheAllConflictOperationsAsync() throws Exception {
+checkCacheAllConflictOperations(true);
+}
+
+/**
+ * @param async boolean flag for asynchronous cache operation processing.
+ */
+private void checkCacheAllConflictOperations(boolean async) throws
Exception {
+int key = 6;
+int val = 1;
+
+GridCacheVersion confl = new GridCacheVersion(1, 0, 1, (byte)2);
+
+Map> putMap = F.asMap(key, new
T3<>(val, confl, CU.EXPIRE_TIME_ETERNAL));
+Map rmvMap = F.asMap(key, confl);
+
+if (async) {
Review Comment:
(!async)
--
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
Re: [PR] IGNITE-23901 Add operation time performance statistics for putAllConflict, removeAllConflict [ignite]
oleg-vlsk commented on code in PR #11793:
URL: https://github.com/apache/ignite/pull/11793#discussion_r1915934895
##
modules/core/src/test/java/org/apache/ignite/internal/processors/performancestatistics/PerformanceStatisticsThinClientTest.java:
##
@@ -171,6 +178,62 @@ public void testCacheOperation() throws Exception {
checkCacheOperation(CACHE_GET_AND_REMOVE, cache ->
cache.getAndRemove(5));
}
+/**
+ * Cache {@link TcpClientCache#putAllConflict} and {@link
TcpClientCache#removeAllConflict} operations performed.
+ * @throws Exception If failed.
+ */
+@Test
+public void testCacheAllConflictOperations() throws Exception {
+checkCacheAllConflictOperations(false);
Review Comment:
Looks much better now, but I believe that moving each individual check into
its own test method would make it even more convenient. Please consider
something like this:
```
public void testPutAllConflict() {
checkCacheOperation(CACHE_PUT_ALL_CONFLICT, cache ->
cache.putAllConflict(getPutMap()));
}
```
for non-async methods with something like for getting your maps:
```
private Map> getPutMap() { return
F.asMap(key, new T3<>(...)); }
private Map getRemoveMap() { return F.asMap(key,
confl); }
```
And maybe something like
```
public void testPutAllConflictOperationsAsync() throws Exception {
runAsyncOp(CACHE_PUT_ALL_CONFLICT);
}
```
For async operations with
```
public void runAsyncOp(OperationType type) {
checkCacheOperation(type, cache -> {
try {
if (type == CACHE_PUT_ALL_CONFLICT)
cache.putAllConflictAsync(getPutMap()).get();
else if (type == CACHE_REMOVE_ALL_CONFLICT)
cache.removeAllConflictAsync(getRemoveMap()).get();
}
catch (InterruptedException | ExecutionException e) {
throw new RuntimeException(e);
}
});
}
```
To avoid code duplication for try-catch.
--
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
Re: [PR] IGNITE-23901 Add operation time performance statistics for putAllConflict, removeAllConflict [ignite]
oleg-vlsk commented on code in PR #11793:
URL: https://github.com/apache/ignite/pull/11793#discussion_r1915934895
##
modules/core/src/test/java/org/apache/ignite/internal/processors/performancestatistics/PerformanceStatisticsThinClientTest.java:
##
@@ -171,6 +178,62 @@ public void testCacheOperation() throws Exception {
checkCacheOperation(CACHE_GET_AND_REMOVE, cache ->
cache.getAndRemove(5));
}
+/**
+ * Cache {@link TcpClientCache#putAllConflict} and {@link
TcpClientCache#removeAllConflict} operations performed.
+ * @throws Exception If failed.
+ */
+@Test
+public void testCacheAllConflictOperations() throws Exception {
+checkCacheAllConflictOperations(false);
Review Comment:
Looks much better now, but I believe that moving each individual check into
its own test method would make it even more convenient. Please consider
something like this:
```
public void testPutAllConflict() {
checkCacheOperation(CACHE_PUT_ALL_CONFLICT, cache ->
cache.putAllConflict(getPutMap()));
}
```
for non-async methods with something like for getting your maps:
```
private Map> getPutMap() { return
F.asMap(key, new T3<>(...)); }
private Map getRemoveMap() { return F.asMap(key,
confl); }
```
And maybe something like
```
public void testPutAllConflictOperationsAsync() throws Exception {
runAsyncOp(CACHE_PUT_ALL_CONFLICT);
}
```
For async operations with
```
public void runAsyncOp(OperationType type) {
checkCacheOperation(type, cache -> {
try {
if (type == CACHE_PUT_ALL_CONFLICT)
cache.putAllConflictAsync(getPutMap()).get();
else if (type == CACHE_REMOVE_ALL_CONFLICT)
cache.removeAllConflictAsync(getRemoveMap()).get();
}
catch (InterruptedException | ExecutionException e) {
throw new RuntimeException(e);
}
});
}
```
To avoid code duplication for try-catch.
--
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
Re: [PR] IGNITE-23901 Add operation time performance statistics for putAllConflict, removeAllConflict [ignite]
oleg-vlsk commented on code in PR #11793:
URL: https://github.com/apache/ignite/pull/11793#discussion_r1915934895
##
modules/core/src/test/java/org/apache/ignite/internal/processors/performancestatistics/PerformanceStatisticsThinClientTest.java:
##
@@ -171,6 +178,62 @@ public void testCacheOperation() throws Exception {
checkCacheOperation(CACHE_GET_AND_REMOVE, cache ->
cache.getAndRemove(5));
}
+/**
+ * Cache {@link TcpClientCache#putAllConflict} and {@link
TcpClientCache#removeAllConflict} operations performed.
+ * @throws Exception If failed.
+ */
+@Test
+public void testCacheAllConflictOperations() throws Exception {
+checkCacheAllConflictOperations(false);
Review Comment:
Looks much better now, but I believe that moving each individual check into
its own test method would make it even more convenient. Please consider
something like this:
```
public void testCacheAllConflict() {
checkCacheOperation(CACHE_PUT_ALL_CONFLICT, cache ->
cache.putAllConflict(getPutMap()));
}
```
for non-async methods with something like for getting your maps:
```
private Map> getPutMap() { return
F.asMap(key, new T3<>(...)); }
private Map getRemoveMap() { return F.asMap(key,
confl); }
```
And maybe something like
```
public void testPutAllConflictOperationsAsync() throws Exception {
runAsyncOp(CACHE_PUT_ALL_CONFLICT);
}
```
For async operations with
```
public void runAsyncOp(OperationType type) {
checkCacheOperation(type, cache -> {
try {
if (type == CACHE_PUT_ALL_CONFLICT)
cache.putAllConflictAsync(getPutMap()).get();
else if (type == CACHE_REMOVE_ALL_CONFLICT)
cache.removeAllConflictAsync(getRemoveMap()).get();
}
catch (InterruptedException | ExecutionException e) {
throw new RuntimeException(e);
}
});
}
```
To avoid code duplication for try-catch.
--
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
Re: [PR] IGNITE-23901 Add operation time performance statistics for putAllConflict, removeAllConflict [ignite]
maksaska commented on code in PR #11793:
URL: https://github.com/apache/ignite/pull/11793#discussion_r1912442492
##
modules/core/src/test/java/org/apache/ignite/internal/processors/performancestatistics/PerformanceStatisticsThinClientTest.java:
##
@@ -199,6 +210,36 @@ private void checkCacheOperation(OperationType op,
Consumer}.
+ */
+private Consumer> putAllConflict(Map map) {
+Map> drMap = new
HashMap<>();
+
+GridCacheVersion confl = new GridCacheVersion(1, 0, 1, (byte)2);
+
+map.forEach((key, value) -> drMap.put(key, new T3<>(value, confl,
CU.EXPIRE_TIME_ETERNAL)));
Review Comment:
I removed redundant map from this method
##
modules/core/src/test/java/org/apache/ignite/internal/processors/performancestatistics/PerformanceStatisticsThinClientTest.java:
##
@@ -199,6 +210,36 @@ private void checkCacheOperation(OperationType op,
Consumer}.
+ */
+private Consumer> putAllConflict(Map map) {
+Map> drMap = new
HashMap<>();
+
+GridCacheVersion confl = new GridCacheVersion(1, 0, 1, (byte)2);
+
+map.forEach((key, value) -> drMap.put(key, new T3<>(value, confl,
CU.EXPIRE_TIME_ETERNAL)));
+
+return cache -> ((TcpClientCache)cache).putAllConflict(drMap);
+}
+
+/**
+ * Cache {@link TcpClientCache#putAllConflict} operation perfomed
+ * @param keys {@link Set} with keys for cache remove all.
+ * @return cache {@link Consumer}.
+ */
+private Consumer>
removeAllConflict(Set keys) {
+Map drMap = new HashMap<>();
+
+GridCacheVersion confl = new GridCacheVersion(1, 0, 1, (byte)2);
+
+keys.forEach(key -> drMap.put(key, confl));
Review Comment:
I removed redundant set from this method
--
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
Re: [PR] IGNITE-23901 Add operation time performance statistics for putAllConflict, removeAllConflict [ignite]
maksaska commented on code in PR #11793:
URL: https://github.com/apache/ignite/pull/11793#discussion_r1912442285
##
modules/core/src/test/java/org/apache/ignite/internal/processors/performancestatistics/PerformanceStatisticsThinClientTest.java:
##
@@ -169,6 +176,10 @@ public void testCacheOperation() throws Exception {
checkCacheOperation(CACHE_REMOVE_ALL, cache ->
cache.removeAll(Collections.singleton(3)));
checkCacheOperation(CACHE_GET_AND_REMOVE, cache ->
cache.getAndRemove(5));
+
+checkCacheOperation(CACHE_PUT_ALL_CONFLICT,
putAllConflict(Collections.singletonMap(6, 1)));
+
+checkCacheOperation(CACHE_REMOVE_ALL_CONFLICT,
removeAllConflict(Collections.singleton(6)));
}
Review Comment:
Added tests
##
modules/core/src/test/java/org/apache/ignite/internal/processors/performancestatistics/PerformanceStatisticsThinClientTest.java:
##
@@ -199,6 +210,36 @@ private void checkCacheOperation(OperationType op,
Consumer}.
+ */
+private Consumer> putAllConflict(Map map) {
+Map> drMap = new
HashMap<>();
+
+GridCacheVersion confl = new GridCacheVersion(1, 0, 1, (byte)2);
+
+map.forEach((key, value) -> drMap.put(key, new T3<>(value, confl,
CU.EXPIRE_TIME_ETERNAL)));
+
+return cache -> ((TcpClientCache)cache).putAllConflict(drMap);
+}
+
+/**
+ * Cache {@link TcpClientCache#putAllConflict} operation perfomed
Review Comment:
Fixed the typo
--
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
Re: [PR] IGNITE-23901 Add operation time performance statistics for putAllConflict, removeAllConflict [ignite]
maksaska commented on code in PR #11793:
URL: https://github.com/apache/ignite/pull/11793#discussion_r1912442227
##
modules/core/src/test/java/org/apache/ignite/internal/processors/performancestatistics/PerformanceStatisticsThinClientTest.java:
##
@@ -169,6 +176,10 @@ public void testCacheOperation() throws Exception {
checkCacheOperation(CACHE_REMOVE_ALL, cache ->
cache.removeAll(Collections.singleton(3)));
checkCacheOperation(CACHE_GET_AND_REMOVE, cache ->
cache.getAndRemove(5));
+
+checkCacheOperation(CACHE_PUT_ALL_CONFLICT,
putAllConflict(Collections.singletonMap(6, 1)));
Review Comment:
Yes, you'r right. There are too many API calls for only one test. I
refactored this part, and moved my code to separete test methods as you
proposed.
As for the second proposal, I see what you mean. Fixed it
##
modules/core/src/test/java/org/apache/ignite/internal/processors/performancestatistics/PerformanceStatisticsThinClientTest.java:
##
@@ -169,6 +176,10 @@ public void testCacheOperation() throws Exception {
checkCacheOperation(CACHE_REMOVE_ALL, cache ->
cache.removeAll(Collections.singleton(3)));
checkCacheOperation(CACHE_GET_AND_REMOVE, cache ->
cache.getAndRemove(5));
+
+checkCacheOperation(CACHE_PUT_ALL_CONFLICT,
putAllConflict(Collections.singletonMap(6, 1)));
+
+checkCacheOperation(CACHE_REMOVE_ALL_CONFLICT,
removeAllConflict(Collections.singleton(6)));
Review Comment:
Fixed it
--
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
Re: [PR] IGNITE-23901 Add operation time performance statistics for putAllConflict, removeAllConflict [ignite]
maksaska commented on code in PR #11793:
URL: https://github.com/apache/ignite/pull/11793#discussion_r1912441015
##
modules/core/src/main/java/org/apache/ignite/internal/processors/performancestatistics/OperationType.java:
##
@@ -91,12 +91,18 @@ public enum OperationType {
QUERY_ROWS(20),
/** Custom query property. */
-QUERY_PROPERTY(21);
+QUERY_PROPERTY(21),
+
+/** Cache put all conflict. */
+CACHE_PUT_ALL_CONFLICT(22),
+
+/** Cache put all conflict. */
Review Comment:
Agree. Fixed it
--
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
Re: [PR] IGNITE-23901 Add operation time performance statistics for putAllConflict, removeAllConflict [ignite]
maksaska commented on code in PR #11793: URL: https://github.com/apache/ignite/pull/11793#discussion_r1912440919 ## modules/core/src/main/java/org/apache/ignite/internal/processors/cache/GridCacheAdapter.java: ## @@ -2871,8 +2883,9 @@ protected IgniteInternalFuture removeAsync0(final K key, @Nullable fina return new GridFinishedFuture(); final boolean statsEnabled = ctx.statisticsEnabled(); +boolean performanceStatsEnabled = ctx.kernalContext().performanceStatistics().enabled(); Review Comment: Agree. Fixed it ## modules/core/src/main/java/org/apache/ignite/internal/processors/cache/GridCacheAdapter.java: ## @@ -2001,8 +2005,9 @@ protected boolean put0(final K key, final V val, final CacheEntryPredicate filte return new GridFinishedFuture(); final boolean statsEnabled = ctx.statisticsEnabled(); +boolean performanceStatsEnabled = ctx.kernalContext().performanceStatistics().enabled(); -long start = statsEnabled ? System.nanoTime() : 0L; +long start = statsEnabled || performanceStatsEnabled ? System.nanoTime() : 0L; Review Comment: Agree. Fixed it ## modules/core/src/main/java/org/apache/ignite/internal/processors/cache/GridCacheAdapter.java: ## @@ -2871,8 +2883,9 @@ protected IgniteInternalFuture removeAsync0(final K key, @Nullable fina return new GridFinishedFuture(); final boolean statsEnabled = ctx.statisticsEnabled(); +boolean performanceStatsEnabled = ctx.kernalContext().performanceStatistics().enabled(); -final long start = statsEnabled ? System.nanoTime() : 0L; +final long start = statsEnabled || performanceStatsEnabled ? System.nanoTime() : 0L; Review Comment: Agree. Fixed it -- 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
Re: [PR] IGNITE-23901 Add operation time performance statistics for putAllConflict, removeAllConflict [ignite]
maksaska commented on code in PR #11793: URL: https://github.com/apache/ignite/pull/11793#discussion_r1912440866 ## modules/core/src/main/java/org/apache/ignite/internal/processors/cache/GridCacheAdapter.java: ## @@ -2001,8 +2005,9 @@ protected boolean put0(final K key, final V val, final CacheEntryPredicate filte return new GridFinishedFuture(); final boolean statsEnabled = ctx.statisticsEnabled(); +boolean performanceStatsEnabled = ctx.kernalContext().performanceStatistics().enabled(); Review Comment: Agree. Fixed it ## modules/core/src/main/java/org/apache/ignite/internal/processors/cache/GridCacheAdapter.java: ## @@ -2845,8 +2853,9 @@ protected IgniteInternalFuture removeAsync0(final K key, @Nullable fina return; boolean statsEnabled = ctx.statisticsEnabled(); +boolean performanceStatsEnabled = ctx.kernalContext().performanceStatistics().enabled(); Review Comment: Agree. Fixed it ## modules/core/src/main/java/org/apache/ignite/internal/processors/cache/GridCacheAdapter.java: ## @@ -2845,8 +2853,9 @@ protected IgniteInternalFuture removeAsync0(final K key, @Nullable fina return; boolean statsEnabled = ctx.statisticsEnabled(); +boolean performanceStatsEnabled = ctx.kernalContext().performanceStatistics().enabled(); -long start = statsEnabled ? System.nanoTime() : 0L; +long start = statsEnabled || performanceStatsEnabled ? System.nanoTime() : 0L; Review Comment: Agree. Fixed it -- 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
Re: [PR] IGNITE-23901 Add operation time performance statistics for putAllConflict, removeAllConflict [ignite]
maksaska commented on code in PR #11793: URL: https://github.com/apache/ignite/pull/11793#discussion_r1912440841 ## modules/core/src/main/java/org/apache/ignite/internal/processors/cache/GridCacheAdapter.java: ## @@ -1975,8 +1975,9 @@ protected boolean put0(final K key, final V val, final CacheEntryPredicate filte return; final boolean statsEnabled = ctx.statisticsEnabled(); +boolean performanceStatsEnabled = ctx.kernalContext().performanceStatistics().enabled(); Review Comment: Agree. Fixed it ## modules/core/src/main/java/org/apache/ignite/internal/processors/cache/GridCacheAdapter.java: ## @@ -1975,8 +1975,9 @@ protected boolean put0(final K key, final V val, final CacheEntryPredicate filte return; final boolean statsEnabled = ctx.statisticsEnabled(); +boolean performanceStatsEnabled = ctx.kernalContext().performanceStatistics().enabled(); -long start = statsEnabled ? System.nanoTime() : 0L; +long start = statsEnabled || performanceStatsEnabled ? System.nanoTime() : 0L; Review Comment: Agree. Fixed it -- 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
Re: [PR] IGNITE-23901 Add operation time performance statistics for putAllConflict, removeAllConflict [ignite]
oleg-vlsk commented on code in PR #11793:
URL: https://github.com/apache/ignite/pull/11793#discussion_r1912379167
##
modules/core/src/test/java/org/apache/ignite/internal/processors/performancestatistics/PerformanceStatisticsThinClientTest.java:
##
@@ -199,6 +210,36 @@ private void checkCacheOperation(OperationType op,
Consumer}.
+ */
+private Consumer> putAllConflict(Map map) {
+Map> drMap = new
HashMap<>();
+
+GridCacheVersion confl = new GridCacheVersion(1, 0, 1, (byte)2);
Review Comment:
Casting to byte looks redundant.
##
modules/core/src/test/java/org/apache/ignite/internal/processors/performancestatistics/PerformanceStatisticsThinClientTest.java:
##
@@ -199,6 +210,36 @@ private void checkCacheOperation(OperationType op,
Consumer}.
+ */
+private Consumer> putAllConflict(Map map) {
+Map> drMap = new
HashMap<>();
+
+GridCacheVersion confl = new GridCacheVersion(1, 0, 1, (byte)2);
+
+map.forEach((key, value) -> drMap.put(key, new T3<>(value, confl,
CU.EXPIRE_TIME_ETERNAL)));
+
+return cache -> ((TcpClientCache)cache).putAllConflict(drMap);
+}
+
+/**
+ * Cache {@link TcpClientCache#putAllConflict} operation perfomed
+ * @param keys {@link Set} with keys for cache remove all.
+ * @return cache {@link Consumer}.
+ */
+private Consumer>
removeAllConflict(Set keys) {
+Map drMap = new HashMap<>();
+
+GridCacheVersion confl = new GridCacheVersion(1, 0, 1, (byte)2);
Review Comment:
Casting to byte looks redundant.
--
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
Re: [PR] IGNITE-23901 Add operation time performance statistics for putAllConflict, removeAllConflict [ignite]
oleg-vlsk commented on code in PR #11793:
URL: https://github.com/apache/ignite/pull/11793#discussion_r1912378543
##
modules/core/src/test/java/org/apache/ignite/internal/processors/performancestatistics/PerformanceStatisticsThinClientTest.java:
##
@@ -199,6 +210,36 @@ private void checkCacheOperation(OperationType op,
Consumer}.
+ */
+private Consumer> putAllConflict(Map map) {
+Map> drMap = new
HashMap<>();
+
+GridCacheVersion confl = new GridCacheVersion(1, 0, 1, (byte)2);
+
+map.forEach((key, value) -> drMap.put(key, new T3<>(value, confl,
CU.EXPIRE_TIME_ETERNAL)));
+
+return cache -> ((TcpClientCache)cache).putAllConflict(drMap);
+}
+
+/**
+ * Cache {@link TcpClientCache#putAllConflict} operation perfomed
Review Comment:
removeAllConflict
--
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
Re: [PR] IGNITE-23901 Add operation time performance statistics for putAllConflict, removeAllConflict [ignite]
oleg-vlsk commented on code in PR #11793:
URL: https://github.com/apache/ignite/pull/11793#discussion_r1912379134
##
modules/core/src/test/java/org/apache/ignite/internal/processors/performancestatistics/PerformanceStatisticsThinClientTest.java:
##
@@ -199,6 +210,36 @@ private void checkCacheOperation(OperationType op,
Consumer}.
+ */
+private Consumer> putAllConflict(Map map) {
+Map> drMap = new
HashMap<>();
+
+GridCacheVersion confl = new GridCacheVersion(1, 0, 1, (byte)2);
+
+map.forEach((key, value) -> drMap.put(key, new T3<>(value, confl,
CU.EXPIRE_TIME_ETERNAL)));
Review Comment:
Using forEach on a singleton map seems unnecessary, maybe something like
```
Map> drMap = F.asMap(
F.firstEntry(map).getKey(),
new T3<>(F.firstValue(map), confl, CU.EXPIRE_TIME_ETERNAL));
```
Again, do we really need a map here as an argument?
--
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
Re: [PR] IGNITE-23901 Add operation time performance statistics for putAllConflict, removeAllConflict [ignite]
oleg-vlsk commented on code in PR #11793:
URL: https://github.com/apache/ignite/pull/11793#discussion_r1912379357
##
modules/core/src/test/java/org/apache/ignite/internal/processors/performancestatistics/PerformanceStatisticsThinClientTest.java:
##
@@ -199,6 +210,36 @@ private void checkCacheOperation(OperationType op,
Consumer}.
+ */
+private Consumer> putAllConflict(Map map) {
+Map> drMap = new
HashMap<>();
+
+GridCacheVersion confl = new GridCacheVersion(1, 0, 1, (byte)2);
+
+map.forEach((key, value) -> drMap.put(key, new T3<>(value, confl,
CU.EXPIRE_TIME_ETERNAL)));
+
+return cache -> ((TcpClientCache)cache).putAllConflict(drMap);
+}
+
+/**
+ * Cache {@link TcpClientCache#putAllConflict} operation perfomed
+ * @param keys {@link Set} with keys for cache remove all.
+ * @return cache {@link Consumer}.
+ */
+private Consumer>
removeAllConflict(Set keys) {
+Map drMap = new HashMap<>();
+
+GridCacheVersion confl = new GridCacheVersion(1, 0, 1, (byte)2);
+
+keys.forEach(key -> drMap.put(key, confl));
Review Comment:
Maybe `Map drMap = F.asMap(F.first(keys),
confl);`?
--
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
Re: [PR] IGNITE-23901 Add operation time performance statistics for putAllConflict, removeAllConflict [ignite]
oleg-vlsk commented on code in PR #11793:
URL: https://github.com/apache/ignite/pull/11793#discussion_r1912379357
##
modules/core/src/test/java/org/apache/ignite/internal/processors/performancestatistics/PerformanceStatisticsThinClientTest.java:
##
@@ -199,6 +210,36 @@ private void checkCacheOperation(OperationType op,
Consumer}.
+ */
+private Consumer> putAllConflict(Map map) {
+Map> drMap = new
HashMap<>();
+
+GridCacheVersion confl = new GridCacheVersion(1, 0, 1, (byte)2);
+
+map.forEach((key, value) -> drMap.put(key, new T3<>(value, confl,
CU.EXPIRE_TIME_ETERNAL)));
+
+return cache -> ((TcpClientCache)cache).putAllConflict(drMap);
+}
+
+/**
+ * Cache {@link TcpClientCache#putAllConflict} operation perfomed
+ * @param keys {@link Set} with keys for cache remove all.
+ * @return cache {@link Consumer}.
+ */
+private Consumer>
removeAllConflict(Set keys) {
+Map drMap = new HashMap<>();
+
+GridCacheVersion confl = new GridCacheVersion(1, 0, 1, (byte)2);
+
+keys.forEach(key -> drMap.put(key, confl));
Review Comment:
Maybe `Map drMap1 = F.asMap(F.first(keys),
confl);`?
--
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
Re: [PR] IGNITE-23901 Add operation time performance statistics for putAllConflict, removeAllConflict [ignite]
oleg-vlsk commented on code in PR #11793:
URL: https://github.com/apache/ignite/pull/11793#discussion_r1912379270
##
modules/core/src/test/java/org/apache/ignite/internal/processors/performancestatistics/PerformanceStatisticsThinClientTest.java:
##
@@ -199,6 +210,36 @@ private void checkCacheOperation(OperationType op,
Consumer}.
+ */
+private Consumer> putAllConflict(Map map) {
+Map> drMap = new
HashMap<>();
+
+GridCacheVersion confl = new GridCacheVersion(1, 0, 1, (byte)2);
+
+map.forEach((key, value) -> drMap.put(key, new T3<>(value, confl,
CU.EXPIRE_TIME_ETERNAL)));
+
+return cache -> ((TcpClientCache)cache).putAllConflict(drMap);
+}
+
+/**
+ * Cache {@link TcpClientCache#putAllConflict} operation perfomed
+ * @param keys {@link Set} with keys for cache remove all.
+ * @return cache {@link Consumer}.
+ */
+private Consumer>
removeAllConflict(Set keys) {
+Map drMap = new HashMap<>();
+
+GridCacheVersion confl = new GridCacheVersion(1, 0, 1, (byte)2);
Review Comment:
Casting to byte looks redundant.
--
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
Re: [PR] IGNITE-23901 Add operation time performance statistics for putAllConflict, removeAllConflict [ignite]
oleg-vlsk commented on code in PR #11793:
URL: https://github.com/apache/ignite/pull/11793#discussion_r1912379167
##
modules/core/src/test/java/org/apache/ignite/internal/processors/performancestatistics/PerformanceStatisticsThinClientTest.java:
##
@@ -199,6 +210,36 @@ private void checkCacheOperation(OperationType op,
Consumer}.
+ */
+private Consumer> putAllConflict(Map map) {
+Map> drMap = new
HashMap<>();
+
+GridCacheVersion confl = new GridCacheVersion(1, 0, 1, (byte)2);
Review Comment:
Casting to byte looks redundant.
--
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
Re: [PR] IGNITE-23901 Add operation time performance statistics for putAllConflict, removeAllConflict [ignite]
oleg-vlsk commented on code in PR #11793:
URL: https://github.com/apache/ignite/pull/11793#discussion_r1912379134
##
modules/core/src/test/java/org/apache/ignite/internal/processors/performancestatistics/PerformanceStatisticsThinClientTest.java:
##
@@ -199,6 +210,36 @@ private void checkCacheOperation(OperationType op,
Consumer}.
+ */
+private Consumer> putAllConflict(Map map) {
+Map> drMap = new
HashMap<>();
+
+GridCacheVersion confl = new GridCacheVersion(1, 0, 1, (byte)2);
+
+map.forEach((key, value) -> drMap.put(key, new T3<>(value, confl,
CU.EXPIRE_TIME_ETERNAL)));
Review Comment:
Using forEach on a singleton map seems unnecessary, maybe something like
```
Map> drMap1 = F.asMap(
F.firstEntry(map).getKey(),
new T3<>(F.firstValue(map), confl, CU.EXPIRE_TIME_ETERNAL));
```
Again, do we really need a map here as an argument?
--
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
Re: [PR] IGNITE-23901 Add operation time performance statistics for putAllConflict, removeAllConflict [ignite]
oleg-vlsk commented on code in PR #11793:
URL: https://github.com/apache/ignite/pull/11793#discussion_r1912378471
##
modules/core/src/test/java/org/apache/ignite/internal/processors/performancestatistics/PerformanceStatisticsThinClientTest.java:
##
@@ -169,6 +176,10 @@ public void testCacheOperation() throws Exception {
checkCacheOperation(CACHE_REMOVE_ALL, cache ->
cache.removeAll(Collections.singleton(3)));
checkCacheOperation(CACHE_GET_AND_REMOVE, cache ->
cache.getAndRemove(5));
+
+checkCacheOperation(CACHE_PUT_ALL_CONFLICT,
putAllConflict(Collections.singletonMap(6, 1)));
+
+checkCacheOperation(CACHE_REMOVE_ALL_CONFLICT,
removeAllConflict(Collections.singleton(6)));
}
Review Comment:
No tests for `putAllConflictAsync` and `removeAllConflictAsync`?
--
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
Re: [PR] IGNITE-23901 Add operation time performance statistics for putAllConflict, removeAllConflict [ignite]
oleg-vlsk commented on code in PR #11793:
URL: https://github.com/apache/ignite/pull/11793#discussion_r1912378430
##
modules/core/src/test/java/org/apache/ignite/internal/processors/performancestatistics/PerformanceStatisticsThinClientTest.java:
##
@@ -169,6 +176,10 @@ public void testCacheOperation() throws Exception {
checkCacheOperation(CACHE_REMOVE_ALL, cache ->
cache.removeAll(Collections.singleton(3)));
checkCacheOperation(CACHE_GET_AND_REMOVE, cache ->
cache.getAndRemove(5));
+
+checkCacheOperation(CACHE_PUT_ALL_CONFLICT,
putAllConflict(Collections.singletonMap(6, 1)));
+
+checkCacheOperation(CACHE_REMOVE_ALL_CONFLICT,
removeAllConflict(Collections.singleton(6)));
Review Comment:
Same for `Collections.singleton(6)`, looks redundant.
--
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
Re: [PR] IGNITE-23901 Add operation time performance statistics for putAllConflict, removeAllConflict [ignite]
oleg-vlsk commented on code in PR #11793:
URL: https://github.com/apache/ignite/pull/11793#discussion_r1912378341
##
modules/core/src/test/java/org/apache/ignite/internal/processors/performancestatistics/PerformanceStatisticsThinClientTest.java:
##
@@ -169,6 +176,10 @@ public void testCacheOperation() throws Exception {
checkCacheOperation(CACHE_REMOVE_ALL, cache ->
cache.removeAll(Collections.singleton(3)));
checkCacheOperation(CACHE_GET_AND_REMOVE, cache ->
cache.getAndRemove(5));
+
+checkCacheOperation(CACHE_PUT_ALL_CONFLICT,
putAllConflict(Collections.singletonMap(6, 1)));
Review Comment:
I suggest extracting these checks into separate test methods. Having too
many checks in a single test reduces readability and maintainability, and makes
it harder to pinpoint the issue when the test fails.
Using a singleton collection seems unnecessary since we're testing just one
key-value pair. While it's essential for previous tests that rely on actual
APIs, here we utilize our own test methods.
--
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
Re: [PR] IGNITE-23901 Add operation time performance statistics for putAllConflict, removeAllConflict [ignite]
oleg-vlsk commented on code in PR #11793:
URL: https://github.com/apache/ignite/pull/11793#discussion_r1912377861
##
modules/core/src/main/java/org/apache/ignite/internal/processors/performancestatistics/OperationType.java:
##
@@ -91,12 +91,18 @@ public enum OperationType {
QUERY_ROWS(20),
/** Custom query property. */
-QUERY_PROPERTY(21);
+QUERY_PROPERTY(21),
+
+/** Cache put all conflict. */
+CACHE_PUT_ALL_CONFLICT(22),
+
+/** Cache put all conflict. */
Review Comment:
remove
--
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
Re: [PR] IGNITE-23901 Add operation time performance statistics for putAllConflict, removeAllConflict [ignite]
oleg-vlsk commented on code in PR #11793: URL: https://github.com/apache/ignite/pull/11793#discussion_r1912377842 ## modules/core/src/main/java/org/apache/ignite/internal/processors/cache/GridCacheAdapter.java: ## @@ -2871,8 +2883,9 @@ protected IgniteInternalFuture removeAsync0(final K key, @Nullable fina return new GridFinishedFuture(); final boolean statsEnabled = ctx.statisticsEnabled(); +boolean performanceStatsEnabled = ctx.kernalContext().performanceStatistics().enabled(); -final long start = statsEnabled ? System.nanoTime() : 0L; +final long start = statsEnabled || performanceStatsEnabled ? System.nanoTime() : 0L; Review Comment: braces? -- 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
Re: [PR] IGNITE-23901 Add operation time performance statistics for putAllConflict, removeAllConflict [ignite]
oleg-vlsk commented on code in PR #11793: URL: https://github.com/apache/ignite/pull/11793#discussion_r1912377802 ## modules/core/src/main/java/org/apache/ignite/internal/processors/cache/GridCacheAdapter.java: ## @@ -2871,8 +2883,9 @@ protected IgniteInternalFuture removeAsync0(final K key, @Nullable fina return new GridFinishedFuture(); final boolean statsEnabled = ctx.statisticsEnabled(); +boolean performanceStatsEnabled = ctx.kernalContext().performanceStatistics().enabled(); Review Comment: final? -- 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
Re: [PR] IGNITE-23901 Add operation time performance statistics for putAllConflict, removeAllConflict [ignite]
oleg-vlsk commented on code in PR #11793: URL: https://github.com/apache/ignite/pull/11793#discussion_r1912377724 ## modules/core/src/main/java/org/apache/ignite/internal/processors/cache/GridCacheAdapter.java: ## @@ -2845,8 +2853,9 @@ protected IgniteInternalFuture removeAsync0(final K key, @Nullable fina return; boolean statsEnabled = ctx.statisticsEnabled(); +boolean performanceStatsEnabled = ctx.kernalContext().performanceStatistics().enabled(); -long start = statsEnabled ? System.nanoTime() : 0L; +long start = statsEnabled || performanceStatsEnabled ? System.nanoTime() : 0L; Review Comment: final? braces for better readability? -- 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
Re: [PR] IGNITE-23901 Add operation time performance statistics for putAllConflict, removeAllConflict [ignite]
oleg-vlsk commented on code in PR #11793: URL: https://github.com/apache/ignite/pull/11793#discussion_r1912377681 ## modules/core/src/main/java/org/apache/ignite/internal/processors/cache/GridCacheAdapter.java: ## @@ -2845,8 +2853,9 @@ protected IgniteInternalFuture removeAsync0(final K key, @Nullable fina return; boolean statsEnabled = ctx.statisticsEnabled(); +boolean performanceStatsEnabled = ctx.kernalContext().performanceStatistics().enabled(); Review Comment: final? -- 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
Re: [PR] IGNITE-23901 Add operation time performance statistics for putAllConflict, removeAllConflict [ignite]
oleg-vlsk commented on code in PR #11793: URL: https://github.com/apache/ignite/pull/11793#discussion_r1912377648 ## modules/core/src/main/java/org/apache/ignite/internal/processors/cache/GridCacheAdapter.java: ## @@ -2001,8 +2005,9 @@ protected boolean put0(final K key, final V val, final CacheEntryPredicate filte return new GridFinishedFuture(); final boolean statsEnabled = ctx.statisticsEnabled(); +boolean performanceStatsEnabled = ctx.kernalContext().performanceStatistics().enabled(); -long start = statsEnabled ? System.nanoTime() : 0L; +long start = statsEnabled || performanceStatsEnabled ? System.nanoTime() : 0L; Review Comment: final? braces for better readability? -- 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
Re: [PR] IGNITE-23901 Add operation time performance statistics for putAllConflict, removeAllConflict [ignite]
oleg-vlsk commented on code in PR #11793: URL: https://github.com/apache/ignite/pull/11793#discussion_r1912377612 ## modules/core/src/main/java/org/apache/ignite/internal/processors/cache/GridCacheAdapter.java: ## @@ -2001,8 +2005,9 @@ protected boolean put0(final K key, final V val, final CacheEntryPredicate filte return new GridFinishedFuture(); final boolean statsEnabled = ctx.statisticsEnabled(); +boolean performanceStatsEnabled = ctx.kernalContext().performanceStatistics().enabled(); Review Comment: final? -- 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
Re: [PR] IGNITE-23901 Add operation time performance statistics for putAllConflict, removeAllConflict [ignite]
oleg-vlsk commented on code in PR #11793: URL: https://github.com/apache/ignite/pull/11793#discussion_r1912377566 ## modules/core/src/main/java/org/apache/ignite/internal/processors/cache/GridCacheAdapter.java: ## @@ -1975,8 +1975,9 @@ protected boolean put0(final K key, final V val, final CacheEntryPredicate filte return; final boolean statsEnabled = ctx.statisticsEnabled(); +boolean performanceStatsEnabled = ctx.kernalContext().performanceStatistics().enabled(); -long start = statsEnabled ? System.nanoTime() : 0L; +long start = statsEnabled || performanceStatsEnabled ? System.nanoTime() : 0L; Review Comment: final? braces for better readability for `statsEnabled || performanceStatsEnabled`? -- 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
Re: [PR] IGNITE-23901 Add operation time performance statistics for putAllConflict, removeAllConflict [ignite]
oleg-vlsk commented on code in PR #11793: URL: https://github.com/apache/ignite/pull/11793#discussion_r1912377480 ## modules/core/src/main/java/org/apache/ignite/internal/processors/cache/GridCacheAdapter.java: ## @@ -1975,8 +1975,9 @@ protected boolean put0(final K key, final V val, final CacheEntryPredicate filte return; final boolean statsEnabled = ctx.statisticsEnabled(); +boolean performanceStatsEnabled = ctx.kernalContext().performanceStatistics().enabled(); Review Comment: final? -- 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
