You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@ignite.apache.org by "maksaska (via GitHub)" <gi...@apache.org> on 2025/01/01 18:05:55 UTC
[PR] IGNITE-23901 Add operation time performance statistics for putAllConflict, removeAllConflict [ignite]
maksaska opened a new pull request, #11793:
URL: https://github.com/apache/ignite/pull/11793
Performance statisitcs tracking added for the following cache operations:
* #putAllConflict()
* #removeAllConflict()
Thank you for submitting the pull request to the Apache Ignite.
In order to streamline the review of the contribution
we ask you to ensure the following steps have been taken:
### The Contribution Checklist
- [ ] There is a single JIRA ticket related to the pull request.
- [ ] The web-link to the pull request is attached to the JIRA ticket.
- [ ] The JIRA ticket has the _Patch Available_ state.
- [ ] The pull request body describes changes that have been made.
The description explains _WHAT_ and _WHY_ was made instead of _HOW_.
- [ ] The pull request title is treated as the final commit message.
The following pattern must be used: `IGNITE-XXXX Change summary` where `XXXX` - number of JIRA issue.
- [ ] A reviewer has been mentioned through the JIRA comments
(see [the Maintainers list](https://cwiki.apache.org/confluence/display/IGNITE/How+to+Contribute#HowtoContribute-ReviewProcessandMaintainers))
- [ ] The pull request has been checked by the Teamcity Bot and
the `green visa` attached to the JIRA ticket (see [TC.Bot: Check PR](https://mtcga.gridgain.com/prs.html))
### Notes
- [How to Contribute](https://cwiki.apache.org/confluence/display/IGNITE/How+to+Contribute)
- [Coding abbreviation rules](https://cwiki.apache.org/confluence/display/IGNITE/Abbreviation+Rules)
- [Coding Guidelines](https://cwiki.apache.org/confluence/display/IGNITE/Coding+Guidelines)
- [Apache Ignite Teamcity Bot](https://cwiki.apache.org/confluence/display/IGNITE/Apache+Ignite+Teamcity+Bot)
If you need any help, please email dev@ignite.apache.org or ask anу advice on http://asf.slack.com _#ignite_ channel.
--
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-unsubscribe@ignite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] IGNITE-23901 Add operation time performance statistics for putAllConflict, removeAllConflict [ignite]
Posted by "oleg-vlsk (via GitHub)" <gi...@apache.org>.
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<?, T3<?, GridCacheVersion, Long>> getPutMap() { return F.asMap(key, new T3<>(...)); }
private Map<?, GridCacheVersion> 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-unsubscribe@ignite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] IGNITE-23901 Add operation time performance statistics for putAllConflict, removeAllConflict [ignite]
Posted by "oleg-vlsk (via GitHub)" <gi...@apache.org>.
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-unsubscribe@ignite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] IGNITE-23901 Add operation time performance statistics for putAllConflict, removeAllConflict [ignite]
Posted by "maksaska (via GitHub)" <gi...@apache.org>.
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<ClientCache<Object,
assertEquals(1, ops.get());
}
+ /**
+ * Cache {@link TcpClientCache#putAllConflict} operation perfomed
+ * @param map {@link Map} with entries for cache put all.
+ * @return cache {@link Consumer<ClientCache>}.
+ */
+ private Consumer<ClientCache<Object, Object>> putAllConflict(Map<Integer, Object> map) {
+ Map<Integer, T3<Object, GridCacheVersion, Long>> 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<Object, Object>)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-unsubscribe@ignite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] IGNITE-23901 Add operation time performance statistics for putAllConflict, removeAllConflict [ignite]
Posted by "NSAmelchev (via GitHub)" <gi...@apache.org>.
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<Object, Object> clientCache = (TcpClientCache<Object, Object>)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-unsubscribe@ignite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] IGNITE-23901 Add operation time performance statistics for putAllConflict, removeAllConflict [ignite]
Posted by "oleg-vlsk (via GitHub)" <gi...@apache.org>.
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<ClientCache<Object,
assertEquals(1, ops.get());
}
+ /**
+ * Cache {@link TcpClientCache#putAllConflict} operation perfomed
+ * @param map {@link Map} with entries for cache put all.
+ * @return cache {@link Consumer<ClientCache>}.
+ */
+ private Consumer<ClientCache<Object, Object>> putAllConflict(Map<Integer, Object> map) {
+ Map<Integer, T3<Object, GridCacheVersion, Long>> 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<Object, Object>)cache).putAllConflict(drMap);
+ }
+
+ /**
+ * Cache {@link TcpClientCache#putAllConflict} operation perfomed
+ * @param keys {@link Set} with keys for cache remove all.
+ * @return cache {@link Consumer<ClientCache>}.
+ */
+ private Consumer<ClientCache<Object, Object>> removeAllConflict(Set<Integer> keys) {
+ Map<Integer, GridCacheVersion> drMap = new HashMap<>();
+
+ GridCacheVersion confl = new GridCacheVersion(1, 0, 1, (byte)2);
+
+ keys.forEach(key -> drMap.put(key, confl));
Review Comment:
Maybe `Map<Integer, GridCacheVersion> 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-unsubscribe@ignite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] IGNITE-23901 Add operation time performance statistics for putAllConflict, removeAllConflict [ignite]
Posted by "oleg-vlsk (via GitHub)" <gi...@apache.org>.
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-unsubscribe@ignite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] IGNITE-23901 Add operation time performance statistics for putAllConflict, removeAllConflict [ignite]
Posted by "maksaska (via GitHub)" <gi...@apache.org>.
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<Object, Object> clientCache = (TcpClientCache<Object, Object>)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-unsubscribe@ignite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] IGNITE-23901 Add operation time performance statistics for putAllConflict, removeAllConflict [ignite]
Posted by "oleg-vlsk (via GitHub)" <gi...@apache.org>.
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-unsubscribe@ignite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] IGNITE-23901 Add operation time performance statistics for putAllConflict, removeAllConflict [ignite]
Posted by "maksaska (via GitHub)" <gi...@apache.org>.
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<?, T3<?, GridCacheVersion, Long>> putMap = F.asMap(key, new T3<>(val, confl, CU.EXPIRE_TIME_ETERNAL));
+ Map<?, GridCacheVersion> 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-unsubscribe@ignite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] IGNITE-23901 Add operation time performance statistics for putAllConflict, removeAllConflict [ignite]
Posted by "maksaska (via GitHub)" <gi...@apache.org>.
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-unsubscribe@ignite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] IGNITE-23901 Add operation time performance statistics for putAllConflict, removeAllConflict [ignite]
Posted by "maksaska (via GitHub)" <gi...@apache.org>.
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-unsubscribe@ignite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] IGNITE-23901 Add operation time performance statistics for putAllConflict, removeAllConflict [ignite]
Posted by "oleg-vlsk (via GitHub)" <gi...@apache.org>.
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<Boolean> removeAsync0(final K key, @Nullable fina
return new GridFinishedFuture<Object>();
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-unsubscribe@ignite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] IGNITE-23901 Add operation time performance statistics for putAllConflict, removeAllConflict [ignite]
Posted by "maksaska (via GitHub)" <gi...@apache.org>.
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<Object>();
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<Boolean> 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<Boolean> 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-unsubscribe@ignite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] IGNITE-23901 Add operation time performance statistics for putAllConflict, removeAllConflict [ignite]
Posted by "maksaska (via GitHub)" <gi...@apache.org>.
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-unsubscribe@ignite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] IGNITE-23901 Add operation time performance statistics for putAllConflict, removeAllConflict [ignite]
Posted by "maksaska (via GitHub)" <gi...@apache.org>.
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-unsubscribe@ignite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] IGNITE-23901 Add operation time performance statistics for putAllConflict, removeAllConflict [ignite]
Posted by "NSAmelchev (via GitHub)" <gi...@apache.org>.
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-unsubscribe@ignite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] IGNITE-23901 Add operation time performance statistics for putAllConflict, removeAllConflict [ignite]
Posted by "maksaska (via GitHub)" <gi...@apache.org>.
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-unsubscribe@ignite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] IGNITE-23901 Add operation time performance statistics for putAllConflict, removeAllConflict [ignite]
Posted by "oleg-vlsk (via GitHub)" <gi...@apache.org>.
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<ClientCache<Object,
assertEquals(1, ops.get());
}
+ /**
+ * Cache {@link TcpClientCache#putAllConflict} operation perfomed
+ * @param map {@link Map} with entries for cache put all.
+ * @return cache {@link Consumer<ClientCache>}.
+ */
+ private Consumer<ClientCache<Object, Object>> putAllConflict(Map<Integer, Object> map) {
+ Map<Integer, T3<Object, GridCacheVersion, Long>> 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<Integer, T3<Object, GridCacheVersion, Long>> 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-unsubscribe@ignite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] IGNITE-23901 Add operation time performance statistics for putAllConflict, removeAllConflict [ignite]
Posted by "oleg-vlsk (via GitHub)" <gi...@apache.org>.
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<?, T3<?, GridCacheVersion, Long>> putMap = F.asMap(key, new T3<>(val, confl, CU.EXPIRE_TIME_ETERNAL));
+ Map<?, GridCacheVersion> 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-unsubscribe@ignite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] IGNITE-23901 Add operation time performance statistics for putAllConflict, removeAllConflict [ignite]
Posted by "oleg-vlsk (via GitHub)" <gi...@apache.org>.
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-unsubscribe@ignite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] IGNITE-23901 Add operation time performance statistics for putAllConflict, removeAllConflict [ignite]
Posted by "oleg-vlsk (via GitHub)" <gi...@apache.org>.
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<?, T3<?, GridCacheVersion, Long>> getPutMap() { return F.asMap(key, new T3<>(...)); }
private Map<?, GridCacheVersion> 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-unsubscribe@ignite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] IGNITE-23901 Add operation time performance statistics for putAllConflict, removeAllConflict [ignite]
Posted by "maksaska (via GitHub)" <gi...@apache.org>.
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<?, T3<?, GridCacheVersion, Long>> putMap = F.asMap(key, new T3<>(val, confl, CU.EXPIRE_TIME_ETERNAL));
+ Map<?, GridCacheVersion> rmvMap = F.asMap(key, confl);
+
+ if (!isAsync) {
+ if (opType.equals(CACHE_PUT_ALL_CONFLICT))
+ checkCacheOperation(opType, cache -> ((TcpClientCache<Object, Object>)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-unsubscribe@ignite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] IGNITE-23901 Add operation time performance statistics for putAllConflict, removeAllConflict [ignite]
Posted by "maksaska (via GitHub)" <gi...@apache.org>.
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<?, T3<?, GridCacheVersion, Long>> 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-unsubscribe@ignite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] IGNITE-23901 Add operation time performance statistics for putAllConflict, removeAllConflict [ignite]
Posted by "oleg-vlsk (via GitHub)" <gi...@apache.org>.
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<Object, Object>)cache).putAllConflict(getPutAllConflictMap(6, 1));
+ else if (opType == CACHE_REMOVE_ALL_CONFLICT && !isAsync)
+ ((TcpClientCache<Object, Object>)cache).removeAllConflict(getRemoveAllConflictMap(6));
+ else if (opType == CACHE_PUT_ALL_CONFLICT)
+ ((TcpClientCache<Object, Object>)cache).putAllConflictAsync(getPutAllConflictMap(7, 2)).get();
+ else if (opType == CACHE_REMOVE_ALL_CONFLICT)
+ ((TcpClientCache<Object, Object>)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<?, T3<?, GridCacheVersion, Long>> 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<Boolean> 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-unsubscribe@ignite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] IGNITE-23901 Add operation time performance statistics for putAllConflict, removeAllConflict [ignite]
Posted by "oleg-vlsk (via GitHub)" <gi...@apache.org>.
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-unsubscribe@ignite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] IGNITE-23901 Add operation time performance statistics for putAllConflict, removeAllConflict [ignite]
Posted by "oleg-vlsk (via GitHub)" <gi...@apache.org>.
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<ClientCache<Object,
assertEquals(1, ops.get());
}
+ /**
+ * Cache {@link TcpClientCache#putAllConflict} operation perfomed
+ * @param map {@link Map} with entries for cache put all.
+ * @return cache {@link Consumer<ClientCache>}.
+ */
+ private Consumer<ClientCache<Object, Object>> putAllConflict(Map<Integer, Object> map) {
+ Map<Integer, T3<Object, GridCacheVersion, Long>> 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<Object, Object>)cache).putAllConflict(drMap);
+ }
+
+ /**
+ * Cache {@link TcpClientCache#putAllConflict} operation perfomed
+ * @param keys {@link Set} with keys for cache remove all.
+ * @return cache {@link Consumer<ClientCache>}.
+ */
+ private Consumer<ClientCache<Object, Object>> removeAllConflict(Set<Integer> keys) {
+ Map<Integer, GridCacheVersion> drMap = new HashMap<>();
+
+ GridCacheVersion confl = new GridCacheVersion(1, 0, 1, (byte)2);
+
+ keys.forEach(key -> drMap.put(key, confl));
Review Comment:
Maybe `Map<Integer, GridCacheVersion> 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-unsubscribe@ignite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] IGNITE-23901 Add operation time performance statistics for putAllConflict, removeAllConflict [ignite]
Posted by "oleg-vlsk (via GitHub)" <gi...@apache.org>.
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-unsubscribe@ignite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] IGNITE-23901 Add operation time performance statistics for putAllConflict, removeAllConflict [ignite]
Posted by "oleg-vlsk (via GitHub)" <gi...@apache.org>.
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<?, T3<?, GridCacheVersion, Long>> getPutMap() { return F.asMap(key, new T3<>(...)); }
private Map<?, GridCacheVersion> 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-unsubscribe@ignite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] IGNITE-23901 Add operation time performance statistics for putAllConflict, removeAllConflict [ignite]
Posted by "oleg-vlsk (via GitHub)" <gi...@apache.org>.
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<ClientCache<Object,
assertEquals(1, ops.get());
}
+ /**
+ * Cache {@link TcpClientCache#putAllConflict} operation perfomed
+ * @param map {@link Map} with entries for cache put all.
+ * @return cache {@link Consumer<ClientCache>}.
+ */
+ private Consumer<ClientCache<Object, Object>> putAllConflict(Map<Integer, Object> map) {
+ Map<Integer, T3<Object, GridCacheVersion, Long>> 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<ClientCache<Object,
assertEquals(1, ops.get());
}
+ /**
+ * Cache {@link TcpClientCache#putAllConflict} operation perfomed
+ * @param map {@link Map} with entries for cache put all.
+ * @return cache {@link Consumer<ClientCache>}.
+ */
+ private Consumer<ClientCache<Object, Object>> putAllConflict(Map<Integer, Object> map) {
+ Map<Integer, T3<Object, GridCacheVersion, Long>> 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<Object, Object>)cache).putAllConflict(drMap);
+ }
+
+ /**
+ * Cache {@link TcpClientCache#putAllConflict} operation perfomed
+ * @param keys {@link Set} with keys for cache remove all.
+ * @return cache {@link Consumer<ClientCache>}.
+ */
+ private Consumer<ClientCache<Object, Object>> removeAllConflict(Set<Integer> keys) {
+ Map<Integer, GridCacheVersion> 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-unsubscribe@ignite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] IGNITE-23901 Add operation time performance statistics for putAllConflict, removeAllConflict [ignite]
Posted by "maksaska (via GitHub)" <gi...@apache.org>.
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<ClientCache<Object,
assertEquals(1, ops.get());
}
+ /**
+ * Cache {@link TcpClientCache#putAllConflict} operation perfomed
+ * @param map {@link Map} with entries for cache put all.
+ * @return cache {@link Consumer<ClientCache>}.
+ */
+ private Consumer<ClientCache<Object, Object>> putAllConflict(Map<Integer, Object> map) {
+ Map<Integer, T3<Object, GridCacheVersion, Long>> 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<ClientCache<Object,
assertEquals(1, ops.get());
}
+ /**
+ * Cache {@link TcpClientCache#putAllConflict} operation perfomed
+ * @param map {@link Map} with entries for cache put all.
+ * @return cache {@link Consumer<ClientCache>}.
+ */
+ private Consumer<ClientCache<Object, Object>> putAllConflict(Map<Integer, Object> map) {
+ Map<Integer, T3<Object, GridCacheVersion, Long>> 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<Object, Object>)cache).putAllConflict(drMap);
+ }
+
+ /**
+ * Cache {@link TcpClientCache#putAllConflict} operation perfomed
+ * @param keys {@link Set} with keys for cache remove all.
+ * @return cache {@link Consumer<ClientCache>}.
+ */
+ private Consumer<ClientCache<Object, Object>> removeAllConflict(Set<Integer> keys) {
+ Map<Integer, GridCacheVersion> 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-unsubscribe@ignite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] IGNITE-23901 Add operation time performance statistics for putAllConflict, removeAllConflict [ignite]
Posted by "maksaska (via GitHub)" <gi...@apache.org>.
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<Boolean> removeAsync0(final K key, @Nullable fina
return new GridFinishedFuture<Object>();
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<Object>();
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<Boolean> removeAsync0(final K key, @Nullable fina
return new GridFinishedFuture<Object>();
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-unsubscribe@ignite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] IGNITE-23901 Add operation time performance statistics for putAllConflict, removeAllConflict [ignite]
Posted by "oleg-vlsk (via GitHub)" <gi...@apache.org>.
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<ClientCache<Object,
assertEquals(1, ops.get());
}
+ /**
+ * Cache {@link TcpClientCache#putAllConflict} operation perfomed
+ * @param map {@link Map} with entries for cache put all.
+ * @return cache {@link Consumer<ClientCache>}.
+ */
+ private Consumer<ClientCache<Object, Object>> putAllConflict(Map<Integer, Object> map) {
+ Map<Integer, T3<Object, GridCacheVersion, Long>> 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<Object, Object>)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-unsubscribe@ignite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] IGNITE-23901 Add operation time performance statistics for putAllConflict, removeAllConflict [ignite]
Posted by "oleg-vlsk (via GitHub)" <gi...@apache.org>.
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<Boolean> 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-unsubscribe@ignite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] IGNITE-23901 Add operation time performance statistics for putAllConflict, removeAllConflict [ignite]
Posted by "oleg-vlsk (via GitHub)" <gi...@apache.org>.
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-unsubscribe@ignite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] IGNITE-23901 Add operation time performance statistics for putAllConflict, removeAllConflict [ignite]
Posted by "maksaska (via GitHub)" <gi...@apache.org>.
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<?, T3<?, GridCacheVersion, Long>> putMap = F.asMap(key, new T3<>(val, confl, CU.EXPIRE_TIME_ETERNAL));
+ Map<?, GridCacheVersion> 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-unsubscribe@ignite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] IGNITE-23901 Add operation time performance statistics for putAllConflict, removeAllConflict [ignite]
Posted by "oleg-vlsk (via GitHub)" <gi...@apache.org>.
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<ClientCache<Object,
assertEquals(1, ops.get());
}
+ /**
+ * Cache {@link TcpClientCache#putAllConflict} operation perfomed
+ * @param map {@link Map} with entries for cache put all.
+ * @return cache {@link Consumer<ClientCache>}.
+ */
+ private Consumer<ClientCache<Object, Object>> putAllConflict(Map<Integer, Object> map) {
+ Map<Integer, T3<Object, GridCacheVersion, Long>> 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-unsubscribe@ignite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] IGNITE-23901 Add operation time performance statistics for putAllConflict, removeAllConflict [ignite]
Posted by "oleg-vlsk (via GitHub)" <gi...@apache.org>.
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<Object>();
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-unsubscribe@ignite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] IGNITE-23901 Add operation time performance statistics for putAllConflict, removeAllConflict [ignite]
Posted by "oleg-vlsk (via GitHub)" <gi...@apache.org>.
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<Object>();
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-unsubscribe@ignite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] IGNITE-23901 Add operation time performance statistics for putAllConflict, removeAllConflict [ignite]
Posted by "oleg-vlsk (via GitHub)" <gi...@apache.org>.
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-unsubscribe@ignite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] IGNITE-23901 Add operation time performance statistics for putAllConflict, removeAllConflict [ignite]
Posted by "maksaska (via GitHub)" <gi...@apache.org>.
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-unsubscribe@ignite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] IGNITE-23901 Add operation time performance statistics for putAllConflict, removeAllConflict [ignite]
Posted by "sonarqubecloud[bot] (via GitHub)" <gi...@apache.org>.
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-unsubscribe@ignite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] IGNITE-23901 Add operation time performance statistics for putAllConflict, removeAllConflict [ignite]
Posted by "maksaska (via GitHub)" <gi...@apache.org>.
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<?, T3<?, GridCacheVersion, Long>> putMap = F.asMap(key, new T3<>(val, confl, CU.EXPIRE_TIME_ETERNAL));
+ Map<?, GridCacheVersion> rmvMap = F.asMap(key, confl);
+
+ if (async) {
+ checkCacheOperation(CACHE_PUT_ALL_CONFLICT, cache -> ((TcpClientCache<Object, Object>)cache).putAllConflict(putMap));
+
+ checkCacheOperation(CACHE_REMOVE_ALL_CONFLICT, cache -> ((TcpClientCache<Object, Object>)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-unsubscribe@ignite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] IGNITE-23901 Add operation time performance statistics for putAllConflict, removeAllConflict [ignite]
Posted by "oleg-vlsk (via GitHub)" <gi...@apache.org>.
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<?, T3<?, GridCacheVersion, Long>> putMap = F.asMap(key, new T3<>(val, confl, CU.EXPIRE_TIME_ETERNAL));
+ Map<?, GridCacheVersion> rmvMap = F.asMap(key, confl);
+
+ if (async) {
+ checkCacheOperation(CACHE_PUT_ALL_CONFLICT, cache -> ((TcpClientCache<Object, Object>)cache).putAllConflict(putMap));
+
+ checkCacheOperation(CACHE_REMOVE_ALL_CONFLICT, cache -> ((TcpClientCache<Object, Object>)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<Object, Object>)cache).putAllConflictAsync(putMap).get();
else if (type == CACHE_REMOVE_ALL_CONFLICT)
((TcpClientCache<Object, Object>)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-unsubscribe@ignite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] IGNITE-23901 Add operation time performance statistics for putAllConflict, removeAllConflict [ignite]
Posted by "maksaska (via GitHub)" <gi...@apache.org>.
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-unsubscribe@ignite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] IGNITE-23901 Add operation time performance statistics for putAllConflict, removeAllConflict [ignite]
Posted by "oleg-vlsk (via GitHub)" <gi...@apache.org>.
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<Boolean> removeAsync0(final K key, @Nullable fina
return new GridFinishedFuture<Object>();
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-unsubscribe@ignite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] IGNITE-23901 Add operation time performance statistics for putAllConflict, removeAllConflict [ignite]
Posted by "oleg-vlsk (via GitHub)" <gi...@apache.org>.
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<Boolean> 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-unsubscribe@ignite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] IGNITE-23901 Add operation time performance statistics for putAllConflict, removeAllConflict [ignite]
Posted by "oleg-vlsk (via GitHub)" <gi...@apache.org>.
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<ClientCache<Object,
assertEquals(1, ops.get());
}
+ /**
+ * Cache {@link TcpClientCache#putAllConflict} operation perfomed
+ * @param map {@link Map} with entries for cache put all.
+ * @return cache {@link Consumer<ClientCache>}.
+ */
+ private Consumer<ClientCache<Object, Object>> putAllConflict(Map<Integer, Object> map) {
+ Map<Integer, T3<Object, GridCacheVersion, Long>> 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<Object, Object>)cache).putAllConflict(drMap);
+ }
+
+ /**
+ * Cache {@link TcpClientCache#putAllConflict} operation perfomed
+ * @param keys {@link Set} with keys for cache remove all.
+ * @return cache {@link Consumer<ClientCache>}.
+ */
+ private Consumer<ClientCache<Object, Object>> removeAllConflict(Set<Integer> keys) {
+ Map<Integer, GridCacheVersion> 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-unsubscribe@ignite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] IGNITE-23901 Add operation time performance statistics for putAllConflict, removeAllConflict [ignite]
Posted by "oleg-vlsk (via GitHub)" <gi...@apache.org>.
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<ClientCache<Object,
assertEquals(1, ops.get());
}
+ /**
+ * Cache {@link TcpClientCache#putAllConflict} operation perfomed
+ * @param map {@link Map} with entries for cache put all.
+ * @return cache {@link Consumer<ClientCache>}.
+ */
+ private Consumer<ClientCache<Object, Object>> putAllConflict(Map<Integer, Object> map) {
+ Map<Integer, T3<Object, GridCacheVersion, Long>> 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<Integer, T3<Object, GridCacheVersion, Long>> 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-unsubscribe@ignite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] IGNITE-23901 Add operation time performance statistics for putAllConflict, removeAllConflict [ignite]
Posted by "oleg-vlsk (via GitHub)" <gi...@apache.org>.
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-unsubscribe@ignite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] IGNITE-23901 Add operation time performance statistics for putAllConflict, removeAllConflict [ignite]
Posted by "oleg-vlsk (via GitHub)" <gi...@apache.org>.
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-unsubscribe@ignite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org