timoninmaxim commented on code in PR #12119:
URL: https://github.com/apache/ignite/pull/12119#discussion_r2128800308
##########
modules/core/src/main/java/org/apache/ignite/internal/IgniteDiagnosticPrepareContext.java:
##########
@@ -255,28 +246,22 @@ public String message() {
/**
* @param msg Message.
- * @param c Closure or {@code null} if only basic info is needed.
+ * @param baseInfo Info or {@code null} if only basic info is needed.
*/
- public void add(String msg, @Nullable DiagnosticBaseClosure c) {
- Object key = c != null ? c.mergeKey() : getClass();
+ public void add(String msg, @Nullable
IgniteDiagnosticMessage.DiagnosticBaseInfo baseInfo) {
+ Object key = baseInfo != null ? baseInfo.mergeKey() : getClass();
- List<String> msgs0 = msgs.get(key);
-
- if (msgs0 == null) {
- msgs0 = new ArrayList<>();
-
- msgs.put(key, msgs0);
- }
+ List<String> msgs0 = msgs.computeIfAbsent(key, k -> new
ArrayList<>());
msgs0.add(msg);
Review Comment:
`add` can be inlined
##########
modules/core/src/main/java/org/apache/ignite/internal/IgniteDiagnosticMessage.java:
##########
@@ -78,22 +79,15 @@ public IgniteDiagnosticMessage() {
}
/**
- * @param marsh Marshaller.
- * @param c Closure to run.
+ * @param reqBytes Marshalled request.
* @param futId Future ID.
* @return Request message.
- * @throws IgniteCheckedException If failed.
*/
- public static IgniteDiagnosticMessage createRequest(Marshaller marsh,
- IgniteClosure<GridKernalContext, IgniteDiagnosticInfo> c,
- long futId
- ) throws IgniteCheckedException {
- byte[] cBytes = U.marshal(marsh, c);
Review Comment:
Let's keep marshalling within this method.
##########
modules/core/src/main/java/org/apache/ignite/internal/IgniteDiagnosticPrepareContext.java:
##########
@@ -255,28 +246,22 @@ public String message() {
/**
* @param msg Message.
- * @param c Closure or {@code null} if only basic info is needed.
+ * @param baseInfo Info or {@code null} if only basic info is needed.
*/
- public void add(String msg, @Nullable DiagnosticBaseClosure c) {
- Object key = c != null ? c.mergeKey() : getClass();
+ public void add(String msg, @Nullable
IgniteDiagnosticMessage.DiagnosticBaseInfo baseInfo) {
Review Comment:
IgniteDiagnosticMessage.DiagnosticBaseInfo -> DiagnosticBaseInfo
##########
modules/core/src/test/java/org/apache/ignite/internal/managers/IgniteDiagnosticMessagesTest.java:
##########
@@ -651,6 +664,12 @@ private void sendDiagnostic() throws Exception {
ctx.basicInfo(dstNode.id(), "Test diagnostic");
+ if (needMoreInfo) {
+ ctx.remoteTxInfo(dstNode.id(), new GridCacheVersion(),
new GridCacheVersion(), "Remote Tx message");
Review Comment:
Let's send meaningful values, and validate them on a receiving side.
##########
modules/core/src/main/java/org/apache/ignite/internal/IgniteDiagnosticPrepareContext.java:
##########
@@ -160,28 +155,31 @@ private void listenAndLog(final IgniteLogger log,
IgniteInternalFuture<String> f
/**
*
*/
- private static final class CompoundInfoClosure implements
IgniteClosure<GridKernalContext, IgniteDiagnosticInfo> {
+ public static final class CompoundInfo implements Serializable {
Review Comment:
Why not Externalizable?
##########
modules/core/src/main/java/org/apache/ignite/internal/IgniteDiagnosticMessage.java:
##########
@@ -298,49 +301,53 @@ public static final class TxEntriesInfoClosure extends
DiagnosticBaseClosure {
}
/** {@inheritDoc} */
- @Override public void merge(DiagnosticBaseClosure other) {
- TxEntriesInfoClosure other0 = (TxEntriesInfoClosure)other;
+ @Override public void merge(DiagnosticBaseInfo other) {
+ TxEntriesInfo other0 = (TxEntriesInfo)other;
assert other0 != null && cacheId == other0.cacheId : other;
this.keys.addAll(other0.keys);
}
- /**
- * @param out Output stream.
- * @throws IOException If failed.
- */
- private void writeObject(java.io.ObjectOutputStream out)
- throws IOException {
- /*
- Transform to List, otherwise Set unmarshalling fails since need
- call KeyCacheObject.finishUnmarshal before adding in Set.
- */
+ /** {@inheritDoc} */
+ @Override public void writeExternal(ObjectOutput out) throws
IOException {
this.keys = new ArrayList<>(keys);
Review Comment:
do you still need this?
##########
modules/core/src/test/java/org/apache/ignite/internal/managers/IgniteDiagnosticMessagesTest.java:
##########
@@ -651,6 +664,12 @@ private void sendDiagnostic() throws Exception {
ctx.basicInfo(dstNode.id(), "Test diagnostic");
+ if (needMoreInfo) {
Review Comment:
Let's send the info unconditionally, you don't need one more test for this
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscr...@ignite.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
