shishkovilja commented on code in PR #12581:
URL: https://github.com/apache/ignite/pull/12581#discussion_r2619717819
##########
modules/core/src/main/java/org/apache/ignite/internal/IgniteDiagnosticRequest.java:
##########
@@ -71,12 +72,12 @@ public IgniteDiagnosticRequest() {
* @param nodeId Node ID.
* @param infos Diagnostic infos.
*/
- public IgniteDiagnosticRequest(long futId, UUID nodeId, @Nullable
Collection<DiagnosticBaseInfo> infos) {
+ public IgniteDiagnosticRequest(long futId, UUID nodeId, @Nullable
LinkedHashSet<DiagnosticBaseInfo> infos) {
Review Comment:
```suggestion
public IgniteDiagnosticRequest(long futId, UUID nodeId, @Nullable
Set<DiagnosticBaseInfo> infos) {
```
##########
modules/core/src/main/java/org/apache/ignite/internal/IgniteDiagnosticRequest.java:
##########
@@ -142,13 +143,13 @@ public void futureId(long futId) {
}
/** @return Compound diagnostic infos. */
- public @Nullable Collection<DiagnosticBaseInfo> infos() {
+ public @Nullable LinkedHashSet<DiagnosticBaseInfo> infos() {
return infos;
}
/** Sets compound diagnostic infos. */
- public void infos(@Nullable Collection<DiagnosticBaseInfo> infos) {
- // Deserialization supports only `Collection` interface in
MessageReader#readCollection.
+ public void infos(@Nullable Set<DiagnosticBaseInfo> infos) {
+ // Deserialization supports only `Set` interface in
MessageReader#readSet.
Review Comment:
Lets's remove redundant comment.
##########
modules/core/src/main/java/org/apache/ignite/internal/direct/stream/DirectByteBufferStream.java:
##########
@@ -1617,7 +1644,7 @@ public <C extends Collection<?>> C
readCollection(MessageCollectionItemType item
if (readSize >= 0) {
if (col == null)
- col = new ArrayList<>(readSize);
+ col = set ? new HashSet<>(readSize, 1.0f) : new
ArrayList<>(readSize);
Review Comment:
Any significant reason not to use `U.newHashSet`?
##########
modules/core/src/main/java/org/apache/ignite/internal/IgniteDiagnosticRequest.java:
##########
@@ -142,13 +143,13 @@ public void futureId(long futId) {
}
/** @return Compound diagnostic infos. */
- public @Nullable Collection<DiagnosticBaseInfo> infos() {
+ public @Nullable LinkedHashSet<DiagnosticBaseInfo> infos() {
Review Comment:
```suggestion
public @Nullable Set<DiagnosticBaseInfo> infos() {
```
##########
modules/core/src/main/java/org/apache/ignite/plugin/extensions/communication/MessageWriter.java:
##########
@@ -292,6 +293,16 @@ public default void setBuffer(ByteBuffer buf) {
*/
public <T> boolean writeCollection(Collection<T> col,
MessageCollectionItemType itemType);
+ /**
+ * Writes set with its elements order.
+ *
+ * @param set Set.
+ * @param itemType Set item type.
+ * @param <T> Type of the objects that set contains.
+ * @return Whether value was fully written.
+ */
+ public <T> boolean writeSet(Set<T> set, MessageCollectionItemType
itemType);
Review Comment:
To remove.
##########
modules/core/src/main/java/org/apache/ignite/internal/processors/cluster/ClusterProcessor.java:
##########
@@ -876,7 +877,7 @@ public IgniteInternalFuture<String>
requestDiagnosticInfo(final UUID nodeId, Ign
*/
private IgniteInternalFuture<String> sendDiagnosticMessage(
UUID nodeId,
- @Nullable Collection<IgniteDiagnosticRequest.DiagnosticBaseInfo> infos
+ @Nullable LinkedHashSet<IgniteDiagnosticRequest.DiagnosticBaseInfo>
infos
Review Comment:
```suggestion
@Nullable Set<IgniteDiagnosticRequest.DiagnosticBaseInfo> infos
```
##########
modules/core/src/main/java/org/apache/ignite/internal/IgniteDiagnosticRequest.java:
##########
@@ -142,13 +143,13 @@ public void futureId(long futId) {
}
/** @return Compound diagnostic infos. */
- public @Nullable Collection<DiagnosticBaseInfo> infos() {
+ public @Nullable LinkedHashSet<DiagnosticBaseInfo> infos() {
return infos;
}
/** Sets compound diagnostic infos. */
- public void infos(@Nullable Collection<DiagnosticBaseInfo> infos) {
- // Deserialization supports only `Collection` interface in
MessageReader#readCollection.
+ public void infos(@Nullable Set<DiagnosticBaseInfo> infos) {
+ // Deserialization supports only `Set` interface in
MessageReader#readSet.
this.infos = toLinkedHashSet(infos);
Review Comment:
```suggestion
this.infos = infos;
```
##########
modules/core/src/main/java/org/apache/ignite/internal/direct/DirectMessageWriter.java:
##########
@@ -345,6 +346,17 @@ public ByteBuffer getBuffer() {
return stream.lastFinished();
}
+ /** {@inheritDoc} */
+ @Override public <T> boolean writeSet(Set<T> set,
MessageCollectionItemType itemType) {
+ DirectByteBufferStream stream = state.item().stream;
+
+ // There is no need to use set under the hood. Elemets are via the
ierator with the order provided.
Review Comment:
I think, this method is redundand. We can always use #writeCollection method.
##########
modules/core/src/main/java/org/apache/ignite/internal/IgniteDiagnosticRequest.java:
##########
@@ -142,13 +143,13 @@ public void futureId(long futId) {
}
/** @return Compound diagnostic infos. */
- public @Nullable Collection<DiagnosticBaseInfo> infos() {
+ public @Nullable LinkedHashSet<DiagnosticBaseInfo> infos() {
return infos;
}
/** Sets compound diagnostic infos. */
- public void infos(@Nullable Collection<DiagnosticBaseInfo> infos) {
- // Deserialization supports only `Collection` interface in
MessageReader#readCollection.
+ public void infos(@Nullable Set<DiagnosticBaseInfo> infos) {
Review Comment:
```suggestion
public void infos(@Nullable Set<DiagnosticBaseInfo> infos) {
```
##########
modules/core/src/main/java/org/apache/ignite/internal/direct/DirectMessageReader.java:
##########
@@ -372,13 +373,24 @@ public ByteBuffer getBuffer() {
@Override public <C extends Collection<?>> C
readCollection(MessageCollectionItemType itemType) {
DirectByteBufferStream stream = state.item().stream;
- C col = stream.readCollection(itemType, this);
+ C col = stream.readList(itemType, this);
lastRead = stream.lastFinished();
return col;
}
+ /** {@inheritDoc} */
+ @Override public <SET extends Set<?>> SET
readSet(MessageCollectionItemType itemType) {
+ DirectByteBufferStream stream = state.item().stream;
+
+ SET set = stream.readSet(itemType, this);
Review Comment:
```suggestion
SET set = stream.readCollection(itemType, this, true);
```
##########
modules/codegen2/src/main/java/org/apache/ignite/internal/MessageSerializerGenerator.java:
##########
@@ -388,7 +388,11 @@ else if (assignableFrom(erasedType(type),
type(Collection.class.getName()))) {
imports.add("org.apache.ignite.plugin.extensions.communication.MessageCollectionItemType");
- returnFalseIfWriteFailed(write, "writer.writeCollection",
getExpr,
+ String collectionWriter = assignableFrom(erasedType(type),
type(Set.class.getName()))
Review Comment:
Let's keep simple `#writeCollection`. Currently, there are no difference,
which collection is written.
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
