Vladsz83 commented on code in PR #12398:
URL: https://github.com/apache/ignite/pull/12398#discussion_r2448171301
##########
modules/core/src/main/java/org/apache/ignite/internal/processors/cache/distributed/GridDistributedTxPrepareResponse.java:
##########
@@ -81,51 +71,24 @@ public GridDistributedTxPrepareResponse(int part,
GridCacheVersion xid, Throwabl
super(xid, 0, addDepInfo);
this.part = part;
- this.err = err;
- }
-
- /**
- * Sets flag mask.
- *
- * @param flag Set or clear.
- * @param mask Mask.
- */
- protected final void setFlag(boolean flag, int mask) {
- flags = flag ? (byte)(flags | mask) : (byte)(flags & ~mask);
- }
-
- /**
- * Reags flag mask.
- *
- * @param mask Mask to read.
- * @return Flag value.
- */
- protected final boolean isFlag(int mask) {
- return (flags & mask) != 0;
+ errMsg = new ErrorMessage(err);
Review Comment:
Suggestion: let's add @`Nullable` to the error type `Throwable.` I see it
stil lcan be null. I vote for using `if(err!=null)` as a trivial optimization.
##########
modules/core/src/main/java/org/apache/ignite/internal/processors/cache/GridCacheIoManager.java:
##########
@@ -775,7 +776,7 @@ private void processFailedMessage(UUID nodeId,
req.miniId(),
req.deployInfo() != null);
- res.error(req.classError());
+ res.errorMessage(new ErrorMessage(req.classError()));
Review Comment:
Did we agree on keeping original-API get/set-error methods? Same below.
##########
modules/core/src/main/java/org/apache/ignite/internal/processors/cache/distributed/GridDistributedTxPrepareResponse.java:
##########
@@ -17,42 +17,32 @@
package org.apache.ignite.internal.processors.cache.distributed;
-import java.nio.ByteBuffer;
-import org.apache.ignite.IgniteCheckedException;
import org.apache.ignite.IgniteLogger;
-import org.apache.ignite.internal.GridDirectTransient;
+import org.apache.ignite.internal.Order;
+import org.apache.ignite.internal.managers.communication.ErrorMessage;
import org.apache.ignite.internal.processors.cache.GridCacheSharedContext;
import org.apache.ignite.internal.processors.cache.transactions.IgniteTxState;
import
org.apache.ignite.internal.processors.cache.transactions.IgniteTxStateAware;
import org.apache.ignite.internal.processors.cache.version.GridCacheVersion;
import org.apache.ignite.internal.util.tostring.GridToStringBuilder;
import org.apache.ignite.internal.util.tostring.GridToStringExclude;
-import org.apache.ignite.internal.util.typedef.internal.U;
-import org.apache.ignite.plugin.extensions.communication.MessageReader;
-import org.apache.ignite.plugin.extensions.communication.MessageWriter;
/**
* Response to prepare request.
*/
public class GridDistributedTxPrepareResponse extends
GridDistributedBaseMessage implements IgniteTxStateAware {
- /** Error. */
+ /** Error message. */
@GridToStringExclude
- @GridDirectTransient
- private Throwable err;
-
- /** Serialized error. */
- private byte[] errBytes;
+ @Order(value = 7, method = "errorMessage")
+ private ErrorMessage errMsg;
/** Transient TX state. */
- @GridDirectTransient
Review Comment:
Do we need these 'Transient' any more? If is without `@Order`, then is
transient.
##########
modules/core/src/main/java/org/apache/ignite/internal/processors/cache/distributed/GridDistributedTxPrepareResponse.java:
##########
@@ -81,51 +71,24 @@ public GridDistributedTxPrepareResponse(int part,
GridCacheVersion xid, Throwabl
super(xid, 0, addDepInfo);
this.part = part;
- this.err = err;
- }
-
- /**
- * Sets flag mask.
- *
- * @param flag Set or clear.
- * @param mask Mask.
- */
- protected final void setFlag(boolean flag, int mask) {
- flags = flag ? (byte)(flags | mask) : (byte)(flags & ~mask);
- }
-
- /**
- * Reags flag mask.
- *
- * @param mask Mask to read.
- * @return Flag value.
- */
- protected final boolean isFlag(int mask) {
- return (flags & mask) != 0;
+ errMsg = new ErrorMessage(err);
}
/** {@inheritDoc} */
@Override public int partition() {
return part;
}
- /** {@inheritDoc} */
- @Override public Throwable error() {
- return err;
- }
-
/**
- * @param err Error to set.
+ * @param part New Partition ID this message is targeted to.
*/
- public void error(Throwable err) {
- this.err = err;
+ public void partition(int part) {
+ this.part = part;
}
- /**
- * @return Rollback flag.
- */
- public boolean isRollback() {
- return err != null;
+ /** {@inheritDoc} */
+ @Override public Throwable error() {
+ return ErrorMessage.error(errMsg);
Review Comment:
`@Nullable Throwable` ?
##########
modules/core/src/main/java/org/apache/ignite/internal/processors/cache/distributed/GridDistributedTxPrepareResponse.java:
##########
@@ -144,104 +107,27 @@ public boolean isRollback() {
}
/** {@inheritDoc} */
- @Override public void prepareMarshal(GridCacheSharedContext<?, ?> ctx)
throws IgniteCheckedException {
- super.prepareMarshal(ctx);
-
- if (err != null && errBytes == null)
- errBytes = U.marshal(ctx, err);
- }
-
- /** {@inheritDoc} */
- @Override public void finishUnmarshal(GridCacheSharedContext<?, ?> ctx,
ClassLoader ldr) throws IgniteCheckedException {
- super.finishUnmarshal(ctx, ldr);
-
- if (errBytes != null && err == null)
- err = U.unmarshal(ctx, errBytes, U.resolveClassLoader(ldr,
ctx.gridConfig()));
- }
-
- /** {@inheritDoc} */
- @Override public boolean writeTo(ByteBuffer buf, MessageWriter writer) {
- writer.setBuffer(buf);
-
- if (!super.writeTo(buf, writer))
- return false;
-
- if (!writer.isHeaderWritten()) {
- if (!writer.writeHeader(directType()))
- return false;
-
- writer.onHeaderWritten();
- }
-
- switch (writer.state()) {
- case 7:
- if (!writer.writeByteArray(errBytes))
- return false;
-
- writer.incrementState();
-
- case 8:
- if (!writer.writeByte(flags))
- return false;
-
- writer.incrementState();
-
- case 9:
- if (!writer.writeInt(part))
- return false;
-
- writer.incrementState();
-
- }
-
- return true;
+ @Override public short directType() {
+ return 26;
}
- /** {@inheritDoc} */
- @Override public boolean readFrom(ByteBuffer buf, MessageReader reader) {
- reader.setBuffer(buf);
-
- if (!super.readFrom(buf, reader))
- return false;
-
- switch (reader.state()) {
- case 7:
- errBytes = reader.readByteArray();
-
- if (!reader.isLastRead())
- return false;
-
- reader.incrementState();
-
- case 8:
- flags = reader.readByte();
-
- if (!reader.isLastRead())
- return false;
-
- reader.incrementState();
-
- case 9:
- part = reader.readInt();
-
- if (!reader.isLastRead())
- return false;
-
- reader.incrementState();
-
- }
-
- return true;
+ /**
+ * @return Error message.
+ */
+ public ErrorMessage errorMessage() {
Review Comment:
`@Nullable ErrorMessage`?
##########
modules/core/src/main/java/org/apache/ignite/internal/processors/cache/distributed/near/GridNearLockFuture.java:
##########
@@ -1245,8 +1245,8 @@ boolean record = retval && oldValTup != null &&
oldValTup.get1().equals(dhtVer);
// returned value if any.
entry.resetFromPrimary(newVal,
lockVer, dhtVer, node.id(), topVer);
- entry.readyNearLock(lockVer,
mappedVer, res.committedVersions(),
- res.rolledbackVersions(),
res.pending());
+ entry.readyNearLock(lockVer,
mappedVer, F.emptyIfNull(res.committedVersions()),
Review Comment:
Suggestion: if these collections can be null, let's put `@Nullable` around
it, the setters and getters.
##########
modules/core/src/main/java/org/apache/ignite/internal/processors/cache/distributed/dht/GridDhtTxPrepareFuture.java:
##########
@@ -776,7 +777,7 @@ private boolean mapIfLocked() {
new CIX1<IgniteInternalFuture<IgniteInternalTx>>() {
@Override public void
applyx(IgniteInternalFuture<IgniteInternalTx> fut) {
if (res.error() == null && fut.error() != null)
- res.error(fut.error());
+ res.errorMessage(new
ErrorMessage(fut.error()));
Review Comment:
The same for keeping previous API get/set-error methods.
##########
modules/core/src/main/java/org/apache/ignite/internal/processors/cache/distributed/near/GridNearTxPrepareResponse.java:
##########
@@ -136,38 +136,46 @@ public GridNearTxPrepareResponse(
this.writeVer = writeVer;
this.retVal = retVal;
this.clientRemapVer = clientRemapVer;
-
- if (onePhaseCommit)
- flags |= NEAR_PREPARE_ONE_PHASE_COMMIT_FLAG_MASK;
+ this.onePhaseCommit = onePhaseCommit;
}
/**
- * @return One-phase commit state on primary node.
+ * @return One-phase commit on primary flag.
*/
public boolean onePhaseCommit() {
- return isFlag(NEAR_PREPARE_ONE_PHASE_COMMIT_FLAG_MASK);
+ return onePhaseCommit;
+ }
+
+ /**
+ * @param onePhaseCommit New one-phase commit on primary flag.
+ */
+ public void onePhaseCommit(boolean onePhaseCommit) {
+ this.onePhaseCommit = onePhaseCommit;
}
/**
- * @return {@code True} if client node should remap transaction.
+ * @return Topology version, which is set when client node should remap
lock request.
*/
- @Nullable AffinityTopologyVersion clientRemapVersion() {
+ @Nullable public AffinityTopologyVersion clientRemapVersion() {
return clientRemapVer;
}
/**
- * Gets pending versions that are less than {@link #version()}.
- *
- * @return Pending versions.
+ * @param clientRemapVer New topology version, which is set when client
node should remap lock request.
+ */
+ public void clientRemapVersion(AffinityTopologyVersion clientRemapVer) {
+ this.clientRemapVer = clientRemapVer;
+ }
+
+ /**
+ * @return Versions that are less than lock version ({@link #version()}).
*/
public Collection<GridCacheVersion> pending() {
- return pending == null ? Collections.emptyList() : pending;
+ return pending;
Review Comment:
Suggesnion: lets add `@Nullable`
##########
modules/core/src/main/java/org/apache/ignite/internal/processors/cache/distributed/near/GridNearTxPrepareResponse.java:
##########
@@ -221,46 +257,85 @@ public void addOwnedValue(IgniteTxKey key,
GridCacheVersion ver, CacheObject val
}
/**
- * @return Owned values map.
+ * @return Map of owned values to set on near node.
*/
public Map<IgniteTxKey, CacheVersionedValue> ownedValues() {
return ownedVals == null ? Collections.emptyMap() :
Collections.unmodifiableMap(ownedVals);
}
/**
- * @return Return value.
+ * @return Cache return value.
*/
public GridCacheReturn returnValue() {
return retVal;
}
/**
- * @param filterFailedKeys Collection of keys that did not pass the filter.
+ * @param retVal New cache return value.
+ */
+ public void returnValue(GridCacheReturn retVal) {
+ this.retVal = retVal;
+ }
+
+ /**
+ * @param filterFailedKeys Keys that did not pass the filter.
*/
public void filterFailedKeys(Collection<IgniteTxKey> filterFailedKeys) {
this.filterFailedKeys = filterFailedKeys;
}
/**
- * @return Collection of keys that did not pass the filter.
+ * @return New keys that did not pass the filter.
*/
public Collection<IgniteTxKey> filterFailedKeys() {
- return filterFailedKeys == null ? Collections.emptyList() :
filterFailedKeys;
+ return filterFailedKeys;
Review Comment:
If it can be null, let's use `@Nullable` here and around.
##########
modules/core/src/main/java/org/apache/ignite/internal/processors/cache/distributed/GridDistributedTxPrepareResponse.java:
##########
@@ -144,104 +107,27 @@ public boolean isRollback() {
}
/** {@inheritDoc} */
- @Override public void prepareMarshal(GridCacheSharedContext<?, ?> ctx)
throws IgniteCheckedException {
- super.prepareMarshal(ctx);
-
- if (err != null && errBytes == null)
- errBytes = U.marshal(ctx, err);
- }
-
- /** {@inheritDoc} */
- @Override public void finishUnmarshal(GridCacheSharedContext<?, ?> ctx,
ClassLoader ldr) throws IgniteCheckedException {
- super.finishUnmarshal(ctx, ldr);
-
- if (errBytes != null && err == null)
- err = U.unmarshal(ctx, errBytes, U.resolveClassLoader(ldr,
ctx.gridConfig()));
- }
-
- /** {@inheritDoc} */
- @Override public boolean writeTo(ByteBuffer buf, MessageWriter writer) {
- writer.setBuffer(buf);
-
- if (!super.writeTo(buf, writer))
- return false;
-
- if (!writer.isHeaderWritten()) {
- if (!writer.writeHeader(directType()))
- return false;
-
- writer.onHeaderWritten();
- }
-
- switch (writer.state()) {
- case 7:
- if (!writer.writeByteArray(errBytes))
- return false;
-
- writer.incrementState();
-
- case 8:
- if (!writer.writeByte(flags))
- return false;
-
- writer.incrementState();
-
- case 9:
- if (!writer.writeInt(part))
- return false;
-
- writer.incrementState();
-
- }
-
- return true;
+ @Override public short directType() {
+ return 26;
}
- /** {@inheritDoc} */
- @Override public boolean readFrom(ByteBuffer buf, MessageReader reader) {
- reader.setBuffer(buf);
-
- if (!super.readFrom(buf, reader))
- return false;
-
- switch (reader.state()) {
- case 7:
- errBytes = reader.readByteArray();
-
- if (!reader.isLastRead())
- return false;
-
- reader.incrementState();
-
- case 8:
- flags = reader.readByte();
-
- if (!reader.isLastRead())
- return false;
-
- reader.incrementState();
-
- case 9:
- part = reader.readInt();
-
- if (!reader.isLastRead())
- return false;
-
- reader.incrementState();
-
- }
-
- return true;
+ /**
+ * @return Error message.
+ */
+ public ErrorMessage errorMessage() {
+ return errMsg;
}
- /** {@inheritDoc} */
- @Override public short directType() {
- return 26;
+ /**
+ * @param errMsg New error message.
+ */
+ public void errorMessage(ErrorMessage errMsg) {
Review Comment:
`@Nullable ErrorMessage errMsg`?
##########
modules/core/src/main/java/org/apache/ignite/internal/processors/cache/distributed/dht/GridDhtTxPrepareFuture.java:
##########
@@ -1951,7 +1953,7 @@ void onResult(GridDhtTxPrepareResponse res) {
boolean rec =
cctx.gridEvents().isRecordable(EVT_CACHE_REBALANCE_OBJECT_LOADED);
- for (GridCacheEntryInfo info : res.preloadEntries()) {
+ for (GridCacheEntryInfo info :
F.emptyIfNull(res.preloadEntries())) {
Review Comment:
If `preloadEntries` can be null, let put `@Nullable` around it, or the
seggers, getters.
--
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]
