IGNITE-27996 One SingleNodeMessage can be serialized numerous times during sending#12824
IGNITE-27996 One SingleNodeMessage can be serialized numerous times during sending#12824zstan wants to merge 1 commit intoapache:masterfrom
Conversation
|
|
||
| case 2: | ||
| if (!writer.writeByteArray(U.toBytes(resp))) | ||
| if (respBytes == null) { |
There was a problem hiding this comment.
Unnecessary '{ }' according to Ignite codestyle.
| } | ||
|
|
||
| return res; | ||
| return Collections.unmodifiableMap(res); |
There was a problem hiding this comment.
Do we need to test the usage of unmodifiable collection here?
|
|
||
| /** {@inheritDoc} */ | ||
| @Override protected void afterTest() { | ||
| stopAllGrids(); |
There was a problem hiding this comment.
Don't we need super.afterTest() here?
| /** */ | ||
| private static class TestDiscoverySpi extends TcpDiscoverySpi { | ||
| /** */ | ||
| private CountDownLatch messageLatch; |
There was a problem hiding this comment.
'Msg' should be used according to Ignite abbreviation standards.
| private CountDownLatch messageLatch; | ||
|
|
||
| /** Message raized trigger. */ | ||
| void messageLatch(CountDownLatch messageLatch) { |
There was a problem hiding this comment.
'Msg' should be used according to Ignite abbreviation standards.
| super( | ||
| ctx, | ||
| TEST_PROCESS, | ||
| (req) -> new GridFinishedFuture<>(req), |
There was a problem hiding this comment.
Method reference may be used here instead of lambda.
|
|
||
| discoSpi.messageLatch(latch); | ||
|
|
||
| Set<UUID> nodeIdsRes = new HashSet<>(); |
There was a problem hiding this comment.
As an option to avoid the for-loop below:
Set<UUID> nodeIdsRes = grid(0).cluster().forServers().nodes().stream().map(ClusterNode::id).collect(Collectors.toSet());
| @Test | ||
| public void testSingleSerializedOnce() throws Exception { | ||
| startGridsMultiThreaded(NODES_CNT); | ||
| startClientGrid(NODES_CNT); |
There was a problem hiding this comment.
We use grid(NODES_CNT) 3 times in the test, maybe it's worth introducing a local variable like this:
IgniteEx client = startClientGrid(NODES_CNT);
|
|
||
| TestRecordingCommunicationSpi clnCommSpi = TestRecordingCommunicationSpi.spi(grid(NODES_CNT)); | ||
|
|
||
| assertTrue(grid(NODES_CNT).configuration().isClientMode()); |
There was a problem hiding this comment.
I would move this line before TestRecordingCommunicationSpi initialization for better readability, unless there's a certain reason why this assertion is done after it?
2 participants
No description provided.