-
Notifications
You must be signed in to change notification settings - Fork 1.9k
IGNITE-28203 Use MarshalableMessage for ErrorMessage #12878
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
7ca597b
e0996f4
3e1b383
41d86b5
4d1d8f7
5271be4
1311312
10064ba
9349d55
5ffcb22
769bd08
2aae0b5
f2ccbe7
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -18,13 +18,17 @@ | |
| package org.apache.ignite.internal.processors.query.calcite.message; | ||
|
|
||
| import java.util.UUID; | ||
| import org.apache.ignite.IgniteCheckedException; | ||
| 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.util.tostring.GridToStringExclude; | ||
| import org.apache.ignite.internal.util.typedef.internal.U; | ||
| import org.jetbrains.annotations.Nullable; | ||
|
|
||
| /** | ||
| * | ||
| */ | ||
| public class CalciteErrorMessage extends ErrorMessage implements CalciteMessage { | ||
| public class CalciteErrorMessage implements CalciteMarshalableMessage { | ||
| /** */ | ||
| @Order(0) | ||
| UUID qryId; | ||
|
|
@@ -33,19 +37,26 @@ public class CalciteErrorMessage extends ErrorMessage implements CalciteMessage | |
| @Order(1) | ||
| long fragmentId; | ||
|
|
||
| /** Error bytes. */ | ||
| @Order(2) | ||
| @GridToStringExclude | ||
| @Nullable public byte[] errBytes; | ||
|
|
||
| /** Error. */ | ||
| private @Nullable Throwable err; | ||
|
|
||
| /** */ | ||
| public CalciteErrorMessage() { | ||
| // No-op. | ||
| } | ||
|
|
||
| /** */ | ||
| public CalciteErrorMessage(UUID qryId, long fragmentId, Throwable err) { | ||
| super(err); | ||
|
|
||
| assert err != null; | ||
|
|
||
| this.qryId = qryId; | ||
| this.fragmentId = fragmentId; | ||
| this.err = err; | ||
| } | ||
|
|
||
| /** | ||
|
|
@@ -62,6 +73,11 @@ public long fragmentId() { | |
| return fragmentId; | ||
| } | ||
|
|
||
| /** */ | ||
| public @Nullable Throwable error() { | ||
| return err; | ||
| } | ||
|
|
||
| /** {@inheritDoc} */ | ||
| @Override public MessageType type() { | ||
| return MessageType.QUERY_ERROR_MESSAGE; | ||
|
|
@@ -71,4 +87,16 @@ public long fragmentId() { | |
| @Override public short directType() { | ||
| return MessageType.QUERY_ERROR_MESSAGE.directType(); | ||
| } | ||
|
|
||
| /** {@inheritDoc} */ | ||
| @Override public void prepareMarshal(GridCacheSharedContext<?, ?> ctx) throws IgniteCheckedException { | ||
| if (err != null) | ||
| errBytes = U.marshal(ctx.marshaller(), err); | ||
| } | ||
|
|
||
| /** {@inheritDoc} */ | ||
| @Override public void prepareUnmarshal(GridCacheSharedContext<?, ?> ctx) throws IgniteCheckedException { | ||
| if (errBytes != null) | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Usually w no
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yes, and the reason is not clear for me. |
||
| err = U.unmarshal(ctx.marshaller(), errBytes, U.resolveClassLoader(ctx.cache().context().gridConfig())); | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. no keed to keep the bytes any more. Less memory consumption
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Just let gc do it work.
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. GC won't deal with alive not nulls. My suggestion is not to consider how long message lives. Just do not keep obsoletes. WDYT?
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'ts a preoptimisation. |
||
| } | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -17,41 +17,26 @@ | |
|
|
||
| package org.apache.ignite.internal.managers.communication; | ||
|
|
||
| import java.io.Serializable; | ||
| import org.apache.ignite.IgniteCheckedException; | ||
| import org.apache.ignite.IgniteException; | ||
| import org.apache.ignite.internal.MessageProcessor; | ||
| import org.apache.ignite.internal.Order; | ||
| import org.apache.ignite.internal.util.tostring.GridToStringExclude; | ||
| import org.apache.ignite.internal.util.typedef.F; | ||
| import org.apache.ignite.internal.util.typedef.internal.S; | ||
| import org.apache.ignite.internal.util.typedef.internal.U; | ||
| import org.apache.ignite.marshaller.jdk.JdkMarshaller; | ||
| import org.apache.ignite.plugin.extensions.communication.Message; | ||
| import org.apache.ignite.plugin.extensions.communication.MessageWriter; | ||
| import org.apache.ignite.marshaller.Marshaller; | ||
| import org.apache.ignite.plugin.extensions.communication.MarshallableMessage; | ||
| import org.jetbrains.annotations.Nullable; | ||
|
|
||
| import static org.apache.ignite.marshaller.Marshallers.jdk; | ||
|
|
||
| /** | ||
| * Message used to transfer {@link Throwable} objects. | ||
| * <p>Because raw serialization of throwables is prohibited, you should use this message when it is necessary | ||
| * to transfer some error as part of some message. See {@link MessageProcessor} for details. | ||
| * <p>Currently, under the hood marshalling and unmarshalling is performed by {@link JdkMarshaller}. | ||
| * <p>If the message serialization fails, wraps this error with own one. | ||
| */ | ||
| @SuppressWarnings({"NullableProblems", "unused"}) | ||
| // TODO Remove Serializable once https://issues.apache.org/jira/browse/IGNITE-27627 is completed. | ||
| public class ErrorMessage implements Message, Serializable { | ||
| /** */ | ||
| private static final long serialVersionUID = 0L; | ||
|
|
||
| /** Serialization and deserealization call holder. */ | ||
| @Order(value = 0, method = "errorBytes") | ||
| public class ErrorMessage implements MarshallableMessage { | ||
| /** Error bytes. */ | ||
| @Order(0) | ||
| @GridToStringExclude | ||
| @Nullable public byte[] errBytes; | ||
|
|
||
| /** Original error. It is transient and necessary only to avoid duplicated serialization and deserializtion. */ | ||
| /** Error. */ | ||
| private @Nullable Throwable err; | ||
|
|
||
| /** | ||
|
|
@@ -62,61 +47,32 @@ public ErrorMessage() { | |
| } | ||
|
|
||
| /** | ||
| * @param err Original error. Will be lazily serialized. | ||
| * @param err Original error. | ||
| */ | ||
| public ErrorMessage(@Nullable Throwable err) { | ||
| this.err = err; | ||
| } | ||
|
|
||
| /** | ||
| * Provides serialized bytes of the error. Should be called only once. | ||
| * | ||
| * @return Serialized error. | ||
| * @see MessageWriter | ||
| */ | ||
| public @Nullable byte[] errorBytes() { | ||
| if (err == null) | ||
| return null; | ||
|
|
||
| /** {@inheritDoc} */ | ||
| @Override public void prepareMarshal(Marshaller marsh) throws IgniteCheckedException { | ||
| try { | ||
| return U.marshal(jdk(), err); | ||
| if (err != null) | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Usually we do |
||
| errBytes = U.marshal(marsh, err); | ||
| } | ||
| catch (IgniteCheckedException e0) { | ||
| catch (IgniteCheckedException e) { | ||
| IgniteCheckedException wrappedErr = new IgniteCheckedException(err.getMessage()); | ||
|
|
||
| wrappedErr.setStackTrace(err.getStackTrace()); | ||
| wrappedErr.addSuppressed(e0); | ||
| wrappedErr.addSuppressed(e); | ||
|
|
||
| try { | ||
| return U.marshal(jdk(), wrappedErr); | ||
| } | ||
| catch (IgniteCheckedException e1) { | ||
| IgniteException marshErr = new IgniteException("Unable to marshal the wrapping error.", e1); | ||
|
|
||
| marshErr.addSuppressed(wrappedErr); | ||
|
|
||
| throw marshErr; | ||
| } | ||
| errBytes = U.marshal(marsh, wrappedErr); | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. no need to keep |
||
| } | ||
| } | ||
|
|
||
| /** | ||
| * Deserializes the error from {@code errBytes}. Should be called only once. | ||
| * | ||
| * @param errBytes Serialized error. | ||
| * @see MessageWriter | ||
| */ | ||
| public void errorBytes(@Nullable byte[] errBytes) { | ||
| if (F.isEmpty(errBytes)) | ||
| err = null; | ||
| else { | ||
| try { | ||
| err = U.unmarshal(jdk(), errBytes, U.gridClassLoader()); | ||
| } | ||
| catch (IgniteCheckedException e) { | ||
| throw new IgniteException("Failed to unmarshal error data bytes.", e); | ||
| } | ||
| } | ||
| /** {@inheritDoc} */ | ||
| @Override public void finishUnmarshal(Marshaller marsh, ClassLoader clsLdr) throws IgniteCheckedException { | ||
| if (errBytes != null) | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Usually we do && err != null. The same in the other places |
||
| err = U.unmarshal(marsh, errBytes, clsLdr); | ||
| } | ||
|
|
||
| /** */ | ||
|
|
@@ -125,8 +81,6 @@ public void errorBytes(@Nullable byte[] errBytes) { | |
| } | ||
|
|
||
| /** | ||
| * Safely gets original error from an error message. | ||
| * | ||
| * @param errorMsg Error message. | ||
| * @return Error containing in the message. | ||
| */ | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -449,7 +449,7 @@ public void resetMetrics() { | |
|
|
||
| List<MessageFactoryProvider> compMsgs = new ArrayList<>(); | ||
|
|
||
| compMsgs.add(new GridIoMessageFactory()); | ||
| compMsgs.add(new GridIoMessageFactory(marsh, U.gridClassLoader())); | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Why not
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Has no answer, unfortunatelly |
||
|
|
||
| for (IgniteComponentType compType : IgniteComponentType.values()) { | ||
| MessageFactoryProvider f = compType.messageFactory(); | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Usually, we do
&& errBytes == nullThere was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, and the reason is not clear for me.
This MUST happen once.