You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@ignite.apache.org by "Vladsz83 (via GitHub)" <gi...@apache.org> on 2025/12/10 18:42:40 UTC
[PR] TcpDiscoveryAbstractTraceableMessage - research [ignite]
Vladsz83 opened a new pull request, #12574:
URL: https://github.com/apache/ignite/pull/12574
(no comment)
--
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-27317 Refactor serialization for TcpDiscoveryAbstractTraceableMessage [ignite]
Posted by "shishkovilja (via GitHub)" <gi...@apache.org>.
shishkovilja commented on code in PR #12574:
URL: https://github.com/apache/ignite/pull/12574#discussion_r2730985243
##########
modules/core/src/main/java/org/apache/ignite/spi/discovery/tcp/messages/TcpDiscoveryAbstractTraceableMessage.java:
##########
@@ -67,6 +74,21 @@ public Object readResolve() {
return this;
}
+ /** @return {@link #spanContainer}'s bytes. */
+ public @Nullable byte[] spanBytes() {
+ return spanContainer == null ? null : spanContainer.serializedSpanBytes();
+ }
+
+ /** @param spanBytes {@link #spanContainer}'s bytes. */
+ public void spanBytes(@Nullable byte[] spanBytes) {
+ if (spanBytes == null)
+ return;
+
+ readResolve();
Review Comment:
`readResolve` ensures, that `SpanContainer` will be **always** initialized during deserialization via Java deserialization.
I suggest to call `readResolve` in the beginning of the 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-27317 Refactor serialization for TcpDiscoveryAbstractTraceableMessage [ignite]
Posted by "shishkovilja (via GitHub)" <gi...@apache.org>.
shishkovilja commented on code in PR #12574:
URL: https://github.com/apache/ignite/pull/12574#discussion_r2731381054
##########
modules/core/src/main/java/org/apache/ignite/spi/discovery/tcp/messages/TcpDiscoveryAbstractTraceableMessage.java:
##########
@@ -67,6 +74,21 @@ public Object readResolve() {
return this;
}
+ /** @return {@link #spanContainer}'s bytes. */
+ public @Nullable byte[] spanBytes() {
+ return spanContainer == null ? null : spanContainer.serializedSpanBytes();
+ }
+
+ /** @param spanBytes {@link #spanContainer}'s bytes. */
+ public void spanBytes(@Nullable byte[] spanBytes) {
+ if (spanBytes == null)
+ return;
+
+ readResolve();
Review Comment:
@Vladsz83 , that's what I am talking about. But now behaviour changed with new framework and java serde is not used.
--
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] TcpDiscoveryAbstractTraceableMessage - research [ignite]
Posted by "shishkovilja (via GitHub)" <gi...@apache.org>.
shishkovilja commented on code in PR #12574:
URL: https://github.com/apache/ignite/pull/12574#discussion_r2694438436
##########
modules/core/src/main/java/org/apache/ignite/spi/discovery/tcp/TcpDiscoveryIoSession.java:
##########
@@ -192,37 +194,26 @@ <T> T readMessage() throws IgniteCheckedException, IOException {
if (read == -1)
throw new EOFException("Connection closed before message was fully read.");
- if (msgBuf.position() > 0) {
- msgBuf.limit(msgBuf.position() + read);
-
- // We've stored an unprocessed tail before.
- msgBuf.rewind();
- }
- else
- msgBuf.limit(read);
+ msgBuf.limit(read);
finished = msgSer.readFrom(msg, msgReader);
- // We rely on the fact that Discovery only sends next message upon receiving a receipt for the previous one.
+ // Server Discovery only sends next message to next Server upon receiving a receipt for the previous one.
// This behaviour guarantees that we never read a next message from the buffer right after the end of
- // the previous message.
- assert msgBuf.remaining() == 0 || !finished : "Some data was read from the socket but left unprocessed.";
-
- if (finished)
- break;
+ // the previous message. But it is not guaranteed with Client Discovery where messages aren't acknowledged.
+ // Thus, we have to keep the uprocessed bytes read from the socket. It won't return them again.
+ if (msgBuf.remaining() > 0) {
Review Comment:
`msgBuffer.hasRemaining()` ?
##########
modules/core/src/main/java/org/apache/ignite/spi/discovery/tcp/TcpDiscoveryIoSession.java:
##########
@@ -192,37 +194,26 @@ <T> T readMessage() throws IgniteCheckedException, IOException {
if (read == -1)
throw new EOFException("Connection closed before message was fully read.");
- if (msgBuf.position() > 0) {
- msgBuf.limit(msgBuf.position() + read);
-
- // We've stored an unprocessed tail before.
- msgBuf.rewind();
- }
- else
- msgBuf.limit(read);
+ msgBuf.limit(read);
finished = msgSer.readFrom(msg, msgReader);
- // We rely on the fact that Discovery only sends next message upon receiving a receipt for the previous one.
+ // Server Discovery only sends next message to next Server upon receiving a receipt for the previous one.
// This behaviour guarantees that we never read a next message from the buffer right after the end of
- // the previous message.
- assert msgBuf.remaining() == 0 || !finished : "Some data was read from the socket but left unprocessed.";
-
- if (finished)
- break;
+ // the previous message. But it is not guaranteed with Client Discovery where messages aren't acknowledged.
+ // Thus, we have to keep the uprocessed bytes read from the socket. It won't return them again.
+ if (msgBuf.remaining() > 0) {
Review Comment:
`msgBuf.hasRemaining()` ?
--
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] TcpDiscoveryAbstractTraceableMessage - research [ignite]
Posted by "wernerdv (via GitHub)" <gi...@apache.org>.
wernerdv commented on code in PR #12574:
URL: https://github.com/apache/ignite/pull/12574#discussion_r2715811406
##########
modules/core/src/main/java/org/apache/ignite/spi/discovery/tcp/messages/TcpDiscoveryAbstractTraceableMessage.java:
##########
@@ -29,8 +31,12 @@ public abstract class TcpDiscoveryAbstractTraceableMessage extends TcpDiscoveryA
/** Container. */
private SpanContainer spanContainer = new SpanContainer();
+ /** Serialization holder of {@link #spanContainer}'s bytes. */
+ @Order(value = 5, method = "spanBytes")
Review Comment:
Please add `@SuppressWarnings("unused")`
--
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-27317 Refactor serialization for TcpDiscoveryAbstractTraceableMessage [ignite]
Posted by "Vladsz83 (via GitHub)" <gi...@apache.org>.
Vladsz83 commented on code in PR #12574:
URL: https://github.com/apache/ignite/pull/12574#discussion_r2731365454
##########
modules/core/src/main/java/org/apache/ignite/spi/discovery/tcp/messages/TcpDiscoveryAbstractTraceableMessage.java:
##########
@@ -67,6 +74,21 @@ public Object readResolve() {
return this;
}
+ /** @return {@link #spanContainer}'s bytes. */
+ public @Nullable byte[] spanBytes() {
+ return spanContainer == null ? null : spanContainer.serializedSpanBytes();
+ }
+
+ /** @param spanBytes {@link #spanContainer}'s bytes. */
+ public void spanBytes(@Nullable byte[] spanBytes) {
+ if (spanBytes == null)
+ return;
+
+ readResolve();
Review Comment:
The java serializatiob, JDKMarshaller calls readResolve() itself. In `ObjectStreamClass`:
`readResolveMethod = getInheritableMethod(cl, "readResolve", null, Object.class)`
--
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] TcpDiscoveryAbstractTraceableMessage - research [ignite]
Posted by "sonarqubecloud[bot] (via GitHub)" <gi...@apache.org>.
sonarqubecloud[bot] commented on PR #12574:
URL: https://github.com/apache/ignite/pull/12574#issuecomment-3783531051
## [](https://sonarcloud.io/dashboard?id=apache_ignite&pullRequest=12574) **Quality Gate passed**
Issues
 [0 New issues](https://sonarcloud.io/project/issues?id=apache_ignite&pullRequest=12574&issueStatuses=OPEN,CONFIRMED&sinceLeakPeriod=true)
 [0 Accepted issues](https://sonarcloud.io/project/issues?id=apache_ignite&pullRequest=12574&issueStatuses=ACCEPTED)
Measures
 [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_ignite&pullRequest=12574&issueStatuses=OPEN,CONFIRMED&sinceLeakPeriod=true)
 [0.0% Coverage on New Code](https://sonarcloud.io/component_measures?id=apache_ignite&pullRequest=12574&metric=new_coverage&view=list)
 [0.0% Duplication on New Code](https://sonarcloud.io/component_measures?id=apache_ignite&pullRequest=12574&metric=new_duplicated_lines_density&view=list)
[See analysis details on SonarQube Cloud](https://sonarcloud.io/dashboard?id=apache_ignite&pullRequest=12574)
--
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] TcpDiscoveryAbstractTraceableMessage - research [ignite]
Posted by "shishkovilja (via GitHub)" <gi...@apache.org>.
shishkovilja commented on code in PR #12574:
URL: https://github.com/apache/ignite/pull/12574#discussion_r2694296672
##########
modules/core/src/main/java/org/apache/ignite/spi/discovery/tcp/TcpDiscoveryIoSession.java:
##########
@@ -192,37 +194,26 @@ <T> T readMessage() throws IgniteCheckedException, IOException {
if (read == -1)
throw new EOFException("Connection closed before message was fully read.");
- if (msgBuf.position() > 0) {
- msgBuf.limit(msgBuf.position() + read);
-
- // We've stored an unprocessed tail before.
- msgBuf.rewind();
- }
- else
- msgBuf.limit(read);
+ msgBuf.limit(read);
finished = msgSer.readFrom(msg, msgReader);
- // We rely on the fact that Discovery only sends next message upon receiving a receipt for the previous one.
+ // Server Discovery only sends next message to next Server upon receiving a receipt for the previous one.
// This behaviour guarantees that we never read a next message from the buffer right after the end of
- // the previous message.
- assert msgBuf.remaining() == 0 || !finished : "Some data was read from the socket but left unprocessed.";
-
- if (finished)
- break;
+ // the previous message. But it is not guaranteed with Client Discovery where messages aren't acknowledged.
+ // Thus, we have to keep the uprocessed bytes read from the socket. It won't return them again.
+ if (msgBuf.remaining() > 0) {
+ byte[] unprocessedReadTail = new byte[msgBuf.remaining()];
- // We must keep the uprocessed bytes read from the socket. It won't return them again.
- byte[] unprocessedTail = null;
+ msgBuf.get(unprocessedReadTail, 0, msgBuf.remaining());
- if (msgBuf.remaining() > 0) {
- unprocessedTail = new byte[msgBuf.remaining()];
- msgBuf.get(unprocessedTail, 0, msgBuf.remaining());
+ in.acceptPrefixBuffer(unprocessedReadTail);
}
- msgBuf.clear();
+ if (finished)
+ break;
- if (unprocessedTail != null)
- msgBuf.put(unprocessedTail);
+ msgBuf.clear();
Review Comment:
Can be moved to start of cycle. Also, we can remove clearing of buffer just before cycle (line 189 now)
--
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-27317 Refactor serialization for TcpDiscoveryAbstractTraceableMessage [ignite]
Posted by "shishkovilja (via GitHub)" <gi...@apache.org>.
shishkovilja merged PR #12574:
URL: https://github.com/apache/ignite/pull/12574
--
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-27317 Refactor serialization for TcpDiscoveryAbstractTraceableMessage [ignite]
Posted by "Vladsz83 (via GitHub)" <gi...@apache.org>.
Vladsz83 commented on code in PR #12574:
URL: https://github.com/apache/ignite/pull/12574#discussion_r2731448855
##########
modules/core/src/main/java/org/apache/ignite/spi/discovery/tcp/messages/TcpDiscoveryAbstractTraceableMessage.java:
##########
@@ -67,6 +74,21 @@ public Object readResolve() {
return this;
}
+ /** @return {@link #spanContainer}'s bytes. */
+ public @Nullable byte[] spanBytes() {
+ return spanContainer == null ? null : spanContainer.serializedSpanBytes();
+ }
+
+ /** @param spanBytes {@link #spanContainer}'s bytes. */
+ public void spanBytes(@Nullable byte[] spanBytes) {
+ if (spanBytes == null)
+ return;
+
+ readResolve();
Review Comment:
With the new serialization we create a message object normally, by a constructor. The field initialization `private SpanContainer spanContainer = new SpanContainer();` should work.
--
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-27317 Refactor serialization for TcpDiscoveryAbstractTraceableMessage [ignite]
Posted by "sonarqubecloud[bot] (via GitHub)" <gi...@apache.org>.
sonarqubecloud[bot] commented on PR #12574:
URL: https://github.com/apache/ignite/pull/12574#issuecomment-3804854946
## [](https://sonarcloud.io/dashboard?id=apache_ignite&pullRequest=12574) **Quality Gate passed**
Issues
 [0 New issues](https://sonarcloud.io/project/issues?id=apache_ignite&pullRequest=12574&issueStatuses=OPEN,CONFIRMED&sinceLeakPeriod=true)
 [0 Accepted issues](https://sonarcloud.io/project/issues?id=apache_ignite&pullRequest=12574&issueStatuses=ACCEPTED)
Measures
 [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_ignite&pullRequest=12574&issueStatuses=OPEN,CONFIRMED&sinceLeakPeriod=true)
 [0.0% Coverage on New Code](https://sonarcloud.io/component_measures?id=apache_ignite&pullRequest=12574&metric=new_coverage&view=list)
 [0.0% Duplication on New Code](https://sonarcloud.io/component_measures?id=apache_ignite&pullRequest=12574&metric=new_duplicated_lines_density&view=list)
[See analysis details on SonarQube Cloud](https://sonarcloud.io/dashboard?id=apache_ignite&pullRequest=12574)
--
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] TcpDiscoveryAbstractTraceableMessage - research [ignite]
Posted by "wernerdv (via GitHub)" <gi...@apache.org>.
wernerdv commented on code in PR #12574:
URL: https://github.com/apache/ignite/pull/12574#discussion_r2715797404
##########
modules/core/src/main/java/org/apache/ignite/spi/discovery/tcp/messages/TcpDiscoveryNodeLeftMessage.java:
##########
@@ -39,6 +46,11 @@ public TcpDiscoveryNodeLeftMessage(UUID creatorNodeId) {
super(creatorNodeId);
}
+ /** {@inheritDoc} */
+ @Override public short directType() {
+ return 14;
Review Comment:
It should be `16`.
--
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-27317 Refactor serialization for TcpDiscoveryAbstractTraceableMessage [ignite]
Posted by "shishkovilja (via GitHub)" <gi...@apache.org>.
shishkovilja commented on code in PR #12574:
URL: https://github.com/apache/ignite/pull/12574#discussion_r2731381054
##########
modules/core/src/main/java/org/apache/ignite/spi/discovery/tcp/messages/TcpDiscoveryAbstractTraceableMessage.java:
##########
@@ -67,6 +74,21 @@ public Object readResolve() {
return this;
}
+ /** @return {@link #spanContainer}'s bytes. */
+ public @Nullable byte[] spanBytes() {
+ return spanContainer == null ? null : spanContainer.serializedSpanBytes();
+ }
+
+ /** @param spanBytes {@link #spanContainer}'s bytes. */
+ public void spanBytes(@Nullable byte[] spanBytes) {
+ if (spanBytes == null)
+ return;
+
+ readResolve();
Review Comment:
@Vladsz83 , that's what I am talking about. But now behaviour changed.
--
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-27317 Refactor serialization for TcpDiscoveryAbstractTraceableMessage [ignite]
Posted by "shishkovilja (via GitHub)" <gi...@apache.org>.
shishkovilja commented on code in PR #12574:
URL: https://github.com/apache/ignite/pull/12574#discussion_r2731510756
##########
modules/core/src/main/java/org/apache/ignite/spi/discovery/tcp/messages/TcpDiscoveryAbstractTraceableMessage.java:
##########
@@ -67,6 +74,21 @@ public Object readResolve() {
return this;
}
+ /** @return {@link #spanContainer}'s bytes. */
+ public @Nullable byte[] spanBytes() {
+ return spanContainer == null ? null : spanContainer.serializedSpanBytes();
+ }
+
+ /** @param spanBytes {@link #spanContainer}'s bytes. */
+ public void spanBytes(@Nullable byte[] spanBytes) {
+ if (spanBytes == null)
+ return;
+
+ readResolve();
Review Comment:
Yes, you are right. We don't need call `readResolve` in this setter at all.
--
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] TcpDiscoveryAbstractTraceableMessage - research [ignite]
Posted by "shishkovilja (via GitHub)" <gi...@apache.org>.
shishkovilja commented on code in PR #12574:
URL: https://github.com/apache/ignite/pull/12574#discussion_r2694299132
##########
modules/core/src/main/java/org/apache/ignite/spi/discovery/tcp/TcpDiscoveryIoSession.java:
##########
@@ -192,37 +194,26 @@ <T> T readMessage() throws IgniteCheckedException, IOException {
if (read == -1)
throw new EOFException("Connection closed before message was fully read.");
- if (msgBuf.position() > 0) {
- msgBuf.limit(msgBuf.position() + read);
-
- // We've stored an unprocessed tail before.
- msgBuf.rewind();
- }
- else
- msgBuf.limit(read);
+ msgBuf.limit(read);
finished = msgSer.readFrom(msg, msgReader);
- // We rely on the fact that Discovery only sends next message upon receiving a receipt for the previous one.
+ // Server Discovery only sends next message to next Server upon receiving a receipt for the previous one.
// This behaviour guarantees that we never read a next message from the buffer right after the end of
- // the previous message.
- assert msgBuf.remaining() == 0 || !finished : "Some data was read from the socket but left unprocessed.";
-
- if (finished)
- break;
+ // the previous message. But it is not guaranteed with Client Discovery where messages aren't acknowledged.
+ // Thus, we have to keep the uprocessed bytes read from the socket. It won't return them again.
+ if (msgBuf.remaining() > 0) {
+ byte[] unprocessedReadTail = new byte[msgBuf.remaining()];
- // We must keep the uprocessed bytes read from the socket. It won't return them again.
- byte[] unprocessedTail = null;
+ msgBuf.get(unprocessedReadTail, 0, msgBuf.remaining());
- if (msgBuf.remaining() > 0) {
- unprocessedTail = new byte[msgBuf.remaining()];
- msgBuf.get(unprocessedTail, 0, msgBuf.remaining());
+ in.acceptPrefixBuffer(unprocessedReadTail);
}
- msgBuf.clear();
+ if (finished)
Review Comment:
Can be removed, if we replace `while(true)` to `while(!finished)`
##########
modules/core/src/main/java/org/apache/ignite/spi/discovery/tcp/TcpDiscoveryIoSession.java:
##########
@@ -192,37 +194,26 @@ <T> T readMessage() throws IgniteCheckedException, IOException {
if (read == -1)
throw new EOFException("Connection closed before message was fully read.");
- if (msgBuf.position() > 0) {
- msgBuf.limit(msgBuf.position() + read);
-
- // We've stored an unprocessed tail before.
- msgBuf.rewind();
- }
- else
- msgBuf.limit(read);
+ msgBuf.limit(read);
finished = msgSer.readFrom(msg, msgReader);
- // We rely on the fact that Discovery only sends next message upon receiving a receipt for the previous one.
+ // Server Discovery only sends next message to next Server upon receiving a receipt for the previous one.
// This behaviour guarantees that we never read a next message from the buffer right after the end of
- // the previous message.
- assert msgBuf.remaining() == 0 || !finished : "Some data was read from the socket but left unprocessed.";
-
- if (finished)
- break;
+ // the previous message. But it is not guaranteed with Client Discovery where messages aren't acknowledged.
+ // Thus, we have to keep the uprocessed bytes read from the socket. It won't return them again.
+ if (msgBuf.remaining() > 0) {
+ byte[] unprocessedReadTail = new byte[msgBuf.remaining()];
- // We must keep the uprocessed bytes read from the socket. It won't return them again.
- byte[] unprocessedTail = null;
+ msgBuf.get(unprocessedReadTail, 0, msgBuf.remaining());
- if (msgBuf.remaining() > 0) {
- unprocessedTail = new byte[msgBuf.remaining()];
- msgBuf.get(unprocessedTail, 0, msgBuf.remaining());
+ in.acceptPrefixBuffer(unprocessedReadTail);
}
- msgBuf.clear();
+ if (finished)
+ break;
- if (unprocessedTail != null)
- msgBuf.put(unprocessedTail);
+ msgBuf.clear();
}
while (true);
Review Comment:
`while (!finished)` ?
##########
modules/core/src/main/java/org/apache/ignite/spi/discovery/tcp/TcpDiscoveryIoSession.java:
##########
@@ -192,37 +194,26 @@ <T> T readMessage() throws IgniteCheckedException, IOException {
if (read == -1)
throw new EOFException("Connection closed before message was fully read.");
- if (msgBuf.position() > 0) {
- msgBuf.limit(msgBuf.position() + read);
-
- // We've stored an unprocessed tail before.
- msgBuf.rewind();
- }
- else
- msgBuf.limit(read);
+ msgBuf.limit(read);
finished = msgSer.readFrom(msg, msgReader);
- // We rely on the fact that Discovery only sends next message upon receiving a receipt for the previous one.
+ // Server Discovery only sends next message to next Server upon receiving a receipt for the previous one.
// This behaviour guarantees that we never read a next message from the buffer right after the end of
- // the previous message.
- assert msgBuf.remaining() == 0 || !finished : "Some data was read from the socket but left unprocessed.";
-
- if (finished)
- break;
+ // the previous message. But it is not guaranteed with Client Discovery where messages aren't acknowledged.
+ // Thus, we have to keep the uprocessed bytes read from the socket. It won't return them again.
+ if (msgBuf.remaining() > 0) {
+ byte[] unprocessedReadTail = new byte[msgBuf.remaining()];
- // We must keep the uprocessed bytes read from the socket. It won't return them again.
- byte[] unprocessedTail = null;
+ msgBuf.get(unprocessedReadTail, 0, msgBuf.remaining());
- if (msgBuf.remaining() > 0) {
- unprocessedTail = new byte[msgBuf.remaining()];
- msgBuf.get(unprocessedTail, 0, msgBuf.remaining());
+ in.acceptPrefixBuffer(unprocessedReadTail);
}
- msgBuf.clear();
+ if (finished)
+ break;
- if (unprocessedTail != null)
- msgBuf.put(unprocessedTail);
+ msgBuf.clear();
Review Comment:
Can be moved to start of cycle. Also, we can remove clearing of bufer just before cycle (line 189 now)
--
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-27317 Refactor serialization for TcpDiscoveryAbstractTraceableMessage [ignite]
Posted by "shishkovilja (via GitHub)" <gi...@apache.org>.
shishkovilja commented on code in PR #12574:
URL: https://github.com/apache/ignite/pull/12574#discussion_r2730985243
##########
modules/core/src/main/java/org/apache/ignite/spi/discovery/tcp/messages/TcpDiscoveryAbstractTraceableMessage.java:
##########
@@ -67,6 +74,21 @@ public Object readResolve() {
return this;
}
+ /** @return {@link #spanContainer}'s bytes. */
+ public @Nullable byte[] spanBytes() {
+ return spanContainer == null ? null : spanContainer.serializedSpanBytes();
+ }
+
+ /** @param spanBytes {@link #spanContainer}'s bytes. */
+ public void spanBytes(@Nullable byte[] spanBytes) {
+ if (spanBytes == null)
+ return;
+
+ readResolve();
Review Comment:
`readResolve` ensures, that `SpanContainer` will be **always** initialized during deserialization via Java Deserialization.
I suggest, to call it in the beginning of 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-27317 Refactor serialization for TcpDiscoveryAbstractTraceableMessage [ignite]
Posted by "shishkovilja (via GitHub)" <gi...@apache.org>.
shishkovilja commented on code in PR #12574:
URL: https://github.com/apache/ignite/pull/12574#discussion_r2731381054
##########
modules/core/src/main/java/org/apache/ignite/spi/discovery/tcp/messages/TcpDiscoveryAbstractTraceableMessage.java:
##########
@@ -67,6 +74,21 @@ public Object readResolve() {
return this;
}
+ /** @return {@link #spanContainer}'s bytes. */
+ public @Nullable byte[] spanBytes() {
+ return spanContainer == null ? null : spanContainer.serializedSpanBytes();
+ }
+
+ /** @param spanBytes {@link #spanContainer}'s bytes. */
+ public void spanBytes(@Nullable byte[] spanBytes) {
+ if (spanBytes == null)
+ return;
+
+ readResolve();
Review Comment:
@Vladsz83 , that's what I am talking about. But now behaviour changed with new framework.
--
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-27317 Refactor serialization for TcpDiscoveryAbstractTraceableMessage [ignite]
Posted by "sonarqubecloud[bot] (via GitHub)" <gi...@apache.org>.
sonarqubecloud[bot] commented on PR #12574:
URL: https://github.com/apache/ignite/pull/12574#issuecomment-3806269363
## [](https://sonarcloud.io/dashboard?id=apache_ignite&pullRequest=12574) **Quality Gate passed**
Issues
 [0 New issues](https://sonarcloud.io/project/issues?id=apache_ignite&pullRequest=12574&issueStatuses=OPEN,CONFIRMED&sinceLeakPeriod=true)
 [0 Accepted issues](https://sonarcloud.io/project/issues?id=apache_ignite&pullRequest=12574&issueStatuses=ACCEPTED)
Measures
 [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_ignite&pullRequest=12574&issueStatuses=OPEN,CONFIRMED&sinceLeakPeriod=true)
 [0.0% Coverage on New Code](https://sonarcloud.io/component_measures?id=apache_ignite&pullRequest=12574&metric=new_coverage&view=list)
 [0.0% Duplication on New Code](https://sonarcloud.io/component_measures?id=apache_ignite&pullRequest=12574&metric=new_duplicated_lines_density&view=list)
[See analysis details on SonarQube Cloud](https://sonarcloud.io/dashboard?id=apache_ignite&pullRequest=12574)
--
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