You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@seata.apache.org by "Bughue (via GitHub)" <gi...@apache.org> on 2023/12/25 10:25:21 UTC
[PR] feature : load SeataSerializer by version [incubator-seata]
Bughue opened a new pull request, #6208:
URL: https://github.com/apache/incubator-seata/pull/6208
<!-- Please make sure you have read and understood the contributing guidelines -->
- [ ] I have registered the PR [changes](../changes).
### Ⅰ. Describe what this PR did
load SeataSerializer by version
### Ⅱ. Does this pull request fix one issue?
<!-- If that, add "fixes #xxx" below in the next line, for example, fixes #97. -->
I'll add it later.
### Ⅲ. Why don't you add test cases (unit test/integration test)?
### Ⅳ. Describe how to verify it
### Ⅴ. Special notes for reviews
--
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: dev-unsubscribe@seata.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@seata.apache.org
For additional commands, e-mail: dev-help@seata.apache.org
Re: [PR] optimize : load SeataSerializer by version [incubator-seata]
Posted by "Bughue (via GitHub)" <gi...@apache.org>.
Bughue commented on code in PR #6208:
URL: https://github.com/apache/incubator-seata/pull/6208#discussion_r1442439517
##########
serializer/seata-serializer-seata/src/main/java/io/seata/serializer/seata/SeataSerializer.java:
##########
@@ -16,52 +16,133 @@
*/
package io.seata.serializer.seata;
-import java.nio.ByteBuffer;
-
import io.netty.buffer.ByteBuf;
import io.netty.buffer.Unpooled;
import io.seata.common.loader.LoadLevel;
+import io.seata.common.loader.Scope;
import io.seata.common.util.BufferUtils;
import io.seata.core.protocol.AbstractMessage;
+import io.seata.core.protocol.ProtocolConstants;
import io.seata.core.serializer.Serializer;
+import java.nio.ByteBuffer;
+
/**
* The Seata codec.
- *
*/
-@LoadLevel(name = "SEATA")
+@LoadLevel(name = "SEATA", scope = Scope.PROTOTYPE)
Review Comment:
因为它的构造方法是带有参数的所以不能是单例，所以上次提过new出来之后会有map管理
Because its constructor method is with parameters so can not be singleton, so as mentioned last time, it will be managed by the map after new obj.
--
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: dev-unsubscribe@seata.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@seata.apache.org
For additional commands, e-mail: dev-help@seata.apache.org
Re: [PR] optimize : load SeataSerializer by version [incubator-seata]
Posted by "Bughue (via GitHub)" <gi...@apache.org>.
Bughue commented on code in PR #6208:
URL: https://github.com/apache/incubator-seata/pull/6208#discussion_r1438081626
##########
core/src/main/java/io/seata/core/rpc/netty/v1/ProtocolV1Encoder.java:
##########
@@ -92,7 +92,7 @@ public void encode(ChannelHandlerContext ctx, Object msg, ByteBuf out) {
if (messageType != ProtocolConstants.MSGTYPE_HEARTBEAT_REQUEST
&& messageType != ProtocolConstants.MSGTYPE_HEARTBEAT_RESPONSE) {
// heartbeat has no body
- Serializer serializer = SerializerServiceLoader.load(SerializerType.getByCode(rpcMessage.getCodec()));
+ Serializer serializer = SerializerServiceLoader.load(SerializerType.getByCode(rpcMessage.getCodec()), ProtocolConstants.VERSION_1);
Review Comment:
here https://github.com/apache/incubator-seata/pull/6226
CompatibleProtocolDecoder will chose ProtocolDecoderV0,and ProtocolDecoderV0 will provide v0 parameter
--
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: dev-unsubscribe@seata.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@seata.apache.org
For additional commands, e-mail: dev-help@seata.apache.org
Re: [PR] optimize : load SeataSerializer by version [incubator-seata]
Posted by "wangliang181230 (via GitHub)" <gi...@apache.org>.
wangliang181230 commented on code in PR #6208:
URL: https://github.com/apache/incubator-seata/pull/6208#discussion_r1437319188
##########
serializer/seata-serializer-seata/src/main/java/io/seata/serializer/seata/SeataSerializer.java:
##########
@@ -20,68 +20,36 @@
import io.netty.buffer.ByteBuf;
import io.netty.buffer.Unpooled;
+import io.seata.common.exception.NotSupportYetException;
import io.seata.common.loader.LoadLevel;
+import io.seata.common.loader.Scope;
import io.seata.common.util.BufferUtils;
import io.seata.core.protocol.AbstractMessage;
+import io.seata.core.protocol.ProtocolConstants;
import io.seata.core.serializer.Serializer;
+import io.seata.serializer.seata.protocol.v1.MessageCodecFactoryV1;
+import io.seata.serializer.seata.protocol.v1.SeataSerializerV1;
Review Comment:
Remove unused imports
--
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: dev-unsubscribe@seata.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@seata.apache.org
For additional commands, e-mail: dev-help@seata.apache.org
Re: [PR] optimize : load SeataSerializer by version [incubator-seata]
Posted by "wangliang181230 (via GitHub)" <gi...@apache.org>.
wangliang181230 commented on code in PR #6208:
URL: https://github.com/apache/incubator-seata/pull/6208#discussion_r1437318579
##########
core/src/main/java/io/seata/core/serializer/SerializerServiceLoader.java:
##########
@@ -20,15 +20,18 @@
import io.seata.common.loader.EnhancedServiceNotFoundException;
import io.seata.common.util.ReflectionUtil;
+import java.util.HashMap;
+import java.util.Map;
+
/**
* The Service Loader for the interface {@link Serializer}
- *
*/
public final class SerializerServiceLoader {
private SerializerServiceLoader() {
}
+ private static Map<String, Serializer> serializerMap = new HashMap<>();
Review Comment:
Missing `final`
--
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: dev-unsubscribe@seata.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@seata.apache.org
For additional commands, e-mail: dev-help@seata.apache.org
Re: [PR] optimize : load SeataSerializer by version [incubator-seata]
Posted by "Bughue (via GitHub)" <gi...@apache.org>.
Bughue commented on code in PR #6208:
URL: https://github.com/apache/incubator-seata/pull/6208#discussion_r1438081626
##########
core/src/main/java/io/seata/core/rpc/netty/v1/ProtocolV1Encoder.java:
##########
@@ -92,7 +92,7 @@ public void encode(ChannelHandlerContext ctx, Object msg, ByteBuf out) {
if (messageType != ProtocolConstants.MSGTYPE_HEARTBEAT_REQUEST
&& messageType != ProtocolConstants.MSGTYPE_HEARTBEAT_RESPONSE) {
// heartbeat has no body
- Serializer serializer = SerializerServiceLoader.load(SerializerType.getByCode(rpcMessage.getCodec()));
+ Serializer serializer = SerializerServiceLoader.load(SerializerType.getByCode(rpcMessage.getCodec()), ProtocolConstants.VERSION_1);
Review Comment:
here https://github.com/apache/incubator-seata/pull/6226
CompatibleProtocolDecoder will chose ProtocolDecoderV0,and ProtocolDecoderV0 will provide v0 parameter
--
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: dev-unsubscribe@seata.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@seata.apache.org
For additional commands, e-mail: dev-help@seata.apache.org
Re: [PR] optimize : load SeataSerializer by version [incubator-seata]
Posted by "funky-eyes (via GitHub)" <gi...@apache.org>.
funky-eyes commented on code in PR #6208:
URL: https://github.com/apache/incubator-seata/pull/6208#discussion_r1442424823
##########
serializer/seata-serializer-seata/src/main/java/io/seata/serializer/seata/SeataSerializer.java:
##########
@@ -16,52 +16,133 @@
*/
package io.seata.serializer.seata;
-import java.nio.ByteBuffer;
-
import io.netty.buffer.ByteBuf;
import io.netty.buffer.Unpooled;
import io.seata.common.loader.LoadLevel;
+import io.seata.common.loader.Scope;
import io.seata.common.util.BufferUtils;
import io.seata.core.protocol.AbstractMessage;
+import io.seata.core.protocol.ProtocolConstants;
import io.seata.core.serializer.Serializer;
+import java.nio.ByteBuffer;
+
/**
* The Seata codec.
- *
*/
-@LoadLevel(name = "SEATA")
+@LoadLevel(name = "SEATA", scope = Scope.PROTOTYPE)
public class SeataSerializer implements Serializer {
+ Serializer versionSeataSerializer;
+
+ Serializer seataSerializerV0 = new SeataSerializerV0();
+ Serializer seataSerializerV1 = new SeataSerializerV1();
+
+ public SeataSerializer(Byte version) {
+ if (version == ProtocolConstants.VERSION_0) {
+ versionSeataSerializer = seataSerializerV0;
+ } else if (version == ProtocolConstants.VERSION_1) {
+ versionSeataSerializer = seataSerializerV1;
+ }
+ if (versionSeataSerializer == null) {
+ throw new UnsupportedOperationException("version is not supported");
+ }
+ }
+
@Override
public <T> byte[] serialize(T t) {
- if (!(t instanceof AbstractMessage)) {
- throw new IllegalArgumentException("AbstractMessage isn't available.");
- }
- AbstractMessage abstractMessage = (AbstractMessage)t;
- //typecode
- short typecode = abstractMessage.getTypeCode();
- //msg codec
- MessageSeataCodec messageCodec = MessageCodecFactory.getMessageCodec(typecode);
- //get empty ByteBuffer
- ByteBuf out = Unpooled.buffer(1024);
- //msg encode
- messageCodec.encode(t, out);
- byte[] body = new byte[out.readableBytes()];
- out.readBytes(body);
-
- //typecode + body
- ByteBuffer byteBuffer = ByteBuffer.allocate(2 + body.length);
- byteBuffer.putShort(typecode);
- byteBuffer.put(body);
-
- BufferUtils.flip(byteBuffer);
- byte[] content = new byte[byteBuffer.limit()];
- byteBuffer.get(content);
- return content;
+ return versionSeataSerializer.serialize(t);
}
@Override
public <T> T deserialize(byte[] bytes) {
+ return versionSeataSerializer.deserialize(bytes);
+ }
+
+
+ static class SeataSerializerV1 implements Serializer {
+
+ private SeataSerializerV1() {
+ }
+
+ @Override
+ public <T> byte[] serialize(T t) {
+ if (!(t instanceof AbstractMessage)) {
+ throw new IllegalArgumentException("AbstractMessage isn't available.");
+ }
+ AbstractMessage abstractMessage = (AbstractMessage) t;
+ //type code
+ short typecode = abstractMessage.getTypeCode();
+ //msg codec
+ MessageSeataCodec messageCodec = MessageCodecFactory.getMessageCodec(typecode, ProtocolConstants.VERSION_1);
+ //get empty ByteBuffer
+ ByteBuf out = Unpooled.buffer(1024);
+ //msg encode
+ messageCodec.encode(t, out);
+ byte[] body = new byte[out.readableBytes()];
+ out.readBytes(body);
+
+ ByteBuffer byteBuffer;
+
+ //typecode + body
+ byteBuffer = ByteBuffer.allocate(2 + body.length);
+ byteBuffer.putShort(typecode);
+ byteBuffer.put(body);
+
+ BufferUtils.flip(byteBuffer);
+ byte[] content = new byte[byteBuffer.limit()];
+ byteBuffer.get(content);
+ return content;
+ }
+
+ @Override
+ public <T> T deserialize(byte[] bytes) {
+ return deserializeByVersion(bytes, ProtocolConstants.VERSION_1);
+
+ }
+ }
+ static class SeataSerializerV0 implements Serializer {
Review Comment:
说错了，是静态内部类单例模式
Wrong, it's a static inner class singleton pattern
--
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: dev-unsubscribe@seata.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@seata.apache.org
For additional commands, e-mail: dev-help@seata.apache.org
Re: [PR] optimize : load SeataSerializer by version [incubator-seata]
Posted by "Bughue (via GitHub)" <gi...@apache.org>.
Bughue commented on code in PR #6208:
URL: https://github.com/apache/incubator-seata/pull/6208#discussion_r1438080647
##########
serializer/seata-serializer-seata/src/main/java/io/seata/serializer/seata/protocol/MergeResultMessageCodec.java:
##########
@@ -24,13 +24,16 @@
import io.seata.core.protocol.AbstractMessage;
import io.seata.core.protocol.AbstractResultMessage;
import io.seata.core.protocol.MergeResultMessage;
+import io.seata.serializer.seata.protocol.v1.MessageCodecFactoryV1;
/**
* The type Merge result message codec.
*
*/
public class MergeResultMessageCodec extends AbstractMessageCodec {
+ protected MessageCodecFactory factory = new MessageCodecFactoryV1();
Review Comment:
ok I will fix it
--
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: dev-unsubscribe@seata.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@seata.apache.org
For additional commands, e-mail: dev-help@seata.apache.org
Re: [PR] optimize : load SeataSerializer by version [incubator-seata]
Posted by "Bughue (via GitHub)" <gi...@apache.org>.
Bughue commented on code in PR #6208:
URL: https://github.com/apache/incubator-seata/pull/6208#discussion_r1442439604
##########
serializer/seata-serializer-seata/src/main/java/io/seata/serializer/seata/SeataSerializer.java:
##########
@@ -16,52 +16,133 @@
*/
package io.seata.serializer.seata;
-import java.nio.ByteBuffer;
-
import io.netty.buffer.ByteBuf;
import io.netty.buffer.Unpooled;
import io.seata.common.loader.LoadLevel;
+import io.seata.common.loader.Scope;
import io.seata.common.util.BufferUtils;
import io.seata.core.protocol.AbstractMessage;
+import io.seata.core.protocol.ProtocolConstants;
import io.seata.core.serializer.Serializer;
+import java.nio.ByteBuffer;
+
/**
* The Seata codec.
- *
*/
-@LoadLevel(name = "SEATA")
+@LoadLevel(name = "SEATA", scope = Scope.PROTOTYPE)
public class SeataSerializer implements Serializer {
+ Serializer versionSeataSerializer;
+
+ Serializer seataSerializerV0 = new SeataSerializerV0();
+ Serializer seataSerializerV1 = new SeataSerializerV1();
+
+ public SeataSerializer(Byte version) {
+ if (version == ProtocolConstants.VERSION_0) {
+ versionSeataSerializer = seataSerializerV0;
+ } else if (version == ProtocolConstants.VERSION_1) {
+ versionSeataSerializer = seataSerializerV1;
+ }
+ if (versionSeataSerializer == null) {
+ throw new UnsupportedOperationException("version is not supported");
+ }
+ }
+
@Override
public <T> byte[] serialize(T t) {
- if (!(t instanceof AbstractMessage)) {
- throw new IllegalArgumentException("AbstractMessage isn't available.");
- }
- AbstractMessage abstractMessage = (AbstractMessage)t;
- //typecode
- short typecode = abstractMessage.getTypeCode();
- //msg codec
- MessageSeataCodec messageCodec = MessageCodecFactory.getMessageCodec(typecode);
- //get empty ByteBuffer
- ByteBuf out = Unpooled.buffer(1024);
- //msg encode
- messageCodec.encode(t, out);
- byte[] body = new byte[out.readableBytes()];
- out.readBytes(body);
-
- //typecode + body
- ByteBuffer byteBuffer = ByteBuffer.allocate(2 + body.length);
- byteBuffer.putShort(typecode);
- byteBuffer.put(body);
-
- BufferUtils.flip(byteBuffer);
- byte[] content = new byte[byteBuffer.limit()];
- byteBuffer.get(content);
- return content;
+ return versionSeataSerializer.serialize(t);
}
@Override
public <T> T deserialize(byte[] bytes) {
+ return versionSeataSerializer.deserialize(bytes);
+ }
+
+
+ static class SeataSerializerV1 implements Serializer {
+
+ private SeataSerializerV1() {
+ }
+
+ @Override
+ public <T> byte[] serialize(T t) {
+ if (!(t instanceof AbstractMessage)) {
+ throw new IllegalArgumentException("AbstractMessage isn't available.");
+ }
+ AbstractMessage abstractMessage = (AbstractMessage) t;
+ //type code
+ short typecode = abstractMessage.getTypeCode();
+ //msg codec
+ MessageSeataCodec messageCodec = MessageCodecFactory.getMessageCodec(typecode, ProtocolConstants.VERSION_1);
+ //get empty ByteBuffer
+ ByteBuf out = Unpooled.buffer(1024);
+ //msg encode
+ messageCodec.encode(t, out);
+ byte[] body = new byte[out.readableBytes()];
+ out.readBytes(body);
+
+ ByteBuffer byteBuffer;
+
+ //typecode + body
+ byteBuffer = ByteBuffer.allocate(2 + body.length);
+ byteBuffer.putShort(typecode);
+ byteBuffer.put(body);
+
+ BufferUtils.flip(byteBuffer);
+ byte[] content = new byte[byteBuffer.limit()];
+ byteBuffer.get(content);
+ return content;
+ }
+
+ @Override
+ public <T> T deserialize(byte[] bytes) {
+ return deserializeByVersion(bytes, ProtocolConstants.VERSION_1);
+
+ }
+ }
+ static class SeataSerializerV0 implements Serializer {
Review Comment:
done
--
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: dev-unsubscribe@seata.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@seata.apache.org
For additional commands, e-mail: dev-help@seata.apache.org
Re: [PR] optimize : load SeataSerializer by version [incubator-seata]
Posted by "funky-eyes (via GitHub)" <gi...@apache.org>.
funky-eyes commented on code in PR #6208:
URL: https://github.com/apache/incubator-seata/pull/6208#discussion_r1449625260
##########
serializer/seata-serializer-seata/src/main/java/io/seata/serializer/seata/SeataSerializer.java:
##########
@@ -16,52 +16,133 @@
*/
package io.seata.serializer.seata;
-import java.nio.ByteBuffer;
-
import io.netty.buffer.ByteBuf;
import io.netty.buffer.Unpooled;
import io.seata.common.loader.LoadLevel;
+import io.seata.common.loader.Scope;
import io.seata.common.util.BufferUtils;
import io.seata.core.protocol.AbstractMessage;
+import io.seata.core.protocol.ProtocolConstants;
import io.seata.core.serializer.Serializer;
+import java.nio.ByteBuffer;
+
/**
* The Seata codec.
- *
*/
-@LoadLevel(name = "SEATA")
+@LoadLevel(name = "SEATA", scope = Scope.PROTOTYPE)
public class SeataSerializer implements Serializer {
+ Serializer versionSeataSerializer;
+
+ Serializer seataSerializerV0 = new SeataSerializerV0();
+ Serializer seataSerializerV1 = new SeataSerializerV1();
Review Comment:
如果version是0，为什么要实例化一个v1？
Why instantiate a v1 if version is 0?
--
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@seata.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@seata.apache.org
For additional commands, e-mail: notifications-help@seata.apache.org
Re: [PR] optimize : load SeataSerializer by version [incubator-seata]
Posted by "Bughue (via GitHub)" <gi...@apache.org>.
Bughue commented on code in PR #6208:
URL: https://github.com/apache/incubator-seata/pull/6208#discussion_r1441244324
##########
serializer/seata-serializer-seata/src/main/java/io/seata/serializer/seata/protocol/v0/SeataSerializerV0.java:
##########
Review Comment:
I'll optimize it later.
--
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: dev-unsubscribe@seata.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@seata.apache.org
For additional commands, e-mail: dev-help@seata.apache.org
Re: [PR] optimize : load SeataSerializer by version [incubator-seata]
Posted by "Bughue (via GitHub)" <gi...@apache.org>.
Bughue commented on code in PR #6208:
URL: https://github.com/apache/incubator-seata/pull/6208#discussion_r1438080524
##########
serializer/seata-serializer-seata/src/main/java/io/seata/serializer/seata/SeataSerializer.java:
##########
@@ -16,72 +16,43 @@
*/
package io.seata.serializer.seata;
-import java.nio.ByteBuffer;
-
-import io.netty.buffer.ByteBuf;
-import io.netty.buffer.Unpooled;
+import com.google.common.collect.ImmutableMap;
import io.seata.common.loader.LoadLevel;
-import io.seata.common.util.BufferUtils;
-import io.seata.core.protocol.AbstractMessage;
+import io.seata.common.loader.Scope;
+import io.seata.core.protocol.ProtocolConstants;
import io.seata.core.serializer.Serializer;
+import io.seata.serializer.seata.protocol.v0.SeataSerializerV0;
+import io.seata.serializer.seata.protocol.v1.SeataSerializerV1;
+
+import java.util.Map;
/**
* The Seata codec.
- *
*/
-@LoadLevel(name = "SEATA")
+@LoadLevel(name = "SEATA", scope = Scope.PROTOTYPE)
public class SeataSerializer implements Serializer {
- @Override
- public <T> byte[] serialize(T t) {
- if (!(t instanceof AbstractMessage)) {
- throw new IllegalArgumentException("AbstractMessage isn't available.");
- }
- AbstractMessage abstractMessage = (AbstractMessage)t;
- //typecode
- short typecode = abstractMessage.getTypeCode();
- //msg codec
- MessageSeataCodec messageCodec = MessageCodecFactory.getMessageCodec(typecode);
- //get empty ByteBuffer
- ByteBuf out = Unpooled.buffer(1024);
- //msg encode
- messageCodec.encode(t, out);
- byte[] body = new byte[out.readableBytes()];
- out.readBytes(body);
+ Serializer versionSeataSerializer;
- //typecode + body
- ByteBuffer byteBuffer = ByteBuffer.allocate(2 + body.length);
- byteBuffer.putShort(typecode);
- byteBuffer.put(body);
+ static Map<Byte, Serializer> VERSION_MAP = ImmutableMap.<Byte, Serializer>builder()
+ .put(ProtocolConstants.VERSION_0, new SeataSerializerV0())
+ .put(ProtocolConstants.VERSION_1, new SeataSerializerV1())
+ .build();
- BufferUtils.flip(byteBuffer);
- byte[] content = new byte[byteBuffer.limit()];
- byteBuffer.get(content);
- return content;
+ public SeataSerializer(Byte version){
+ versionSeataSerializer = VERSION_MAP.get(version);
Review Comment:
I will fix it
--
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: dev-unsubscribe@seata.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@seata.apache.org
For additional commands, e-mail: dev-help@seata.apache.org
Re: [PR] optimize : load SeataSerializer by version [incubator-seata]
Posted by "codecov-commenter (via GitHub)" <gi...@apache.org>.
codecov-commenter commented on PR #6208:
URL: https://github.com/apache/incubator-seata/pull/6208#issuecomment-1873589981
## [Codecov](https://app.codecov.io/gh/apache/incubator-seata/pull/6208?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) Report
Attention: `60 lines` in your changes are missing coverage. Please review.
> Comparison is base [(`f3b27de`)](https://app.codecov.io/gh/apache/incubator-seata/commit/f3b27de7725a1faeeeb7dc5501190b765d59377a?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) 49.04% compared to head [(`48a6090`)](https://app.codecov.io/gh/apache/incubator-seata/pull/6208?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) 48.92%.
> Report is 1 commits behind head on 2.x.
<details><summary>Additional details and impacted files</summary>
[](https://app.codecov.io/gh/apache/incubator-seata/pull/6208?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)
```diff
@@ Coverage Diff @@
## 2.x #6208 +/- ##
============================================
- Coverage 49.04% 48.92% -0.13%
+ Complexity 4786 4783 -3
============================================
Files 915 917 +2
Lines 31896 31958 +62
Branches 3855 3862 +7
============================================
- Hits 15644 15636 -8
- Misses 14697 14756 +59
- Partials 1555 1566 +11
```
| [Files](https://app.codecov.io/gh/apache/incubator-seata/pull/6208?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | Coverage Δ | |
|---|---|---|
| [...java/io/seata/core/protocol/ProtocolConstants.java](https://app.codecov.io/gh/apache/incubator-seata/pull/6208?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-Y29yZS9zcmMvbWFpbi9qYXZhL2lvL3NlYXRhL2NvcmUvcHJvdG9jb2wvUHJvdG9jb2xDb25zdGFudHMuamF2YQ==) | `0.00% <ø> (ø)` | |
| [...alizer/seata/protocol/BatchResultMessageCodec.java](https://app.codecov.io/gh/apache/incubator-seata/pull/6208?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-c2VyaWFsaXplci9zZWF0YS1zZXJpYWxpemVyLXNlYXRhL3NyYy9tYWluL2phdmEvaW8vc2VhdGEvc2VyaWFsaXplci9zZWF0YS9wcm90b2NvbC9CYXRjaFJlc3VsdE1lc3NhZ2VDb2RlYy5qYXZh) | `85.45% <100.00%> (+0.54%)` | :arrow_up: |
| [...alizer/seata/protocol/MergeResultMessageCodec.java](https://app.codecov.io/gh/apache/incubator-seata/pull/6208?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-c2VyaWFsaXplci9zZWF0YS1zZXJpYWxpemVyLXNlYXRhL3NyYy9tYWluL2phdmEvaW8vc2VhdGEvc2VyaWFsaXplci9zZWF0YS9wcm90b2NvbC9NZXJnZVJlc3VsdE1lc3NhZ2VDb2RlYy5qYXZh) | `80.48% <100.00%> (+1.00%)` | :arrow_up: |
| [...ializer/seata/protocol/MergedWarpMessageCodec.java](https://app.codecov.io/gh/apache/incubator-seata/pull/6208?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-c2VyaWFsaXplci9zZWF0YS1zZXJpYWxpemVyLXNlYXRhL3NyYy9tYWluL2phdmEvaW8vc2VhdGEvc2VyaWFsaXplci9zZWF0YS9wcm90b2NvbC9NZXJnZWRXYXJwTWVzc2FnZUNvZGVjLmphdmE=) | `83.92% <100.00%> (+0.59%)` | :arrow_up: |
| [.../io/seata/core/rpc/netty/v1/ProtocolV1Decoder.java](https://app.codecov.io/gh/apache/incubator-seata/pull/6208?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-Y29yZS9zcmMvbWFpbi9qYXZhL2lvL3NlYXRhL2NvcmUvcnBjL25ldHR5L3YxL1Byb3RvY29sVjFEZWNvZGVyLmphdmE=) | `0.00% <0.00%> (ø)` | |
| [.../io/seata/core/rpc/netty/v1/ProtocolV1Encoder.java](https://app.codecov.io/gh/apache/incubator-seata/pull/6208?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-Y29yZS9zcmMvbWFpbi9qYXZhL2lvL3NlYXRhL2NvcmUvcnBjL25ldHR5L3YxL1Byb3RvY29sVjFFbmNvZGVyLmphdmE=) | `0.00% <0.00%> (ø)` | |
| [...io/seata/serializer/seata/MessageCodecFactory.java](https://app.codecov.io/gh/apache/incubator-seata/pull/6208?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-c2VyaWFsaXplci9zZWF0YS1zZXJpYWxpemVyLXNlYXRhL3NyYy9tYWluL2phdmEvaW8vc2VhdGEvc2VyaWFsaXplci9zZWF0YS9NZXNzYWdlQ29kZWNGYWN0b3J5LmphdmE=) | `83.18% <83.33%> (ø)` | |
| [...ava/io/seata/serializer/seata/SeataSerializer.java](https://app.codecov.io/gh/apache/incubator-seata/pull/6208?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-c2VyaWFsaXplci9zZWF0YS1zZXJpYWxpemVyLXNlYXRhL3NyYy9tYWluL2phdmEvaW8vc2VhdGEvc2VyaWFsaXplci9zZWF0YS9TZWF0YVNlcmlhbGl6ZXIuamF2YQ==) | `50.00% <50.00%> (-30.00%)` | :arrow_down: |
| [...erializer/seata/protocol/v1/SeataSerializerV1.java](https://app.codecov.io/gh/apache/incubator-seata/pull/6208?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-c2VyaWFsaXplci9zZWF0YS1zZXJpYWxpemVyLXNlYXRhL3NyYy9tYWluL2phdmEvaW8vc2VhdGEvc2VyaWFsaXplci9zZWF0YS9wcm90b2NvbC92MS9TZWF0YVNlcmlhbGl6ZXJWMS5qYXZh) | `80.64% <80.64%> (ø)` | |
| [...seata/core/serializer/SerializerServiceLoader.java](https://app.codecov.io/gh/apache/incubator-seata/pull/6208?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-Y29yZS9zcmMvbWFpbi9qYXZhL2lvL3NlYXRhL2NvcmUvc2VyaWFsaXplci9TZXJpYWxpemVyU2VydmljZUxvYWRlci5qYXZh) | `0.00% <0.00%> (ø)` | |
| ... and [1 more](https://app.codecov.io/gh/apache/incubator-seata/pull/6208?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | |
... and [6 files with indirect coverage changes](https://app.codecov.io/gh/apache/incubator-seata/pull/6208/indirect-changes?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)
</details>
--
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: dev-unsubscribe@seata.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@seata.apache.org
For additional commands, e-mail: dev-help@seata.apache.org
Re: [PR] feature : load SeataSerializer by version [incubator-seata]
Posted by "funky-eyes (via GitHub)" <gi...@apache.org>.
funky-eyes commented on code in PR #6208:
URL: https://github.com/apache/incubator-seata/pull/6208#discussion_r1436778416
##########
serializer/seata-serializer-seata/src/main/java/io/seata/serializer/seata/SeataSerializer.java:
##########
@@ -20,38 +20,58 @@
import io.netty.buffer.ByteBuf;
import io.netty.buffer.Unpooled;
+import io.seata.common.exception.NotSupportYetException;
import io.seata.common.loader.LoadLevel;
+import io.seata.common.loader.Scope;
import io.seata.common.util.BufferUtils;
import io.seata.core.protocol.AbstractMessage;
+import io.seata.core.protocol.ProtocolConstants;
import io.seata.core.serializer.Serializer;
+import io.seata.serializer.seata.protocol.v1.MessageCodecFactoryV1;
/**
* The Seata codec.
*
*/
-@LoadLevel(name = "SEATA")
+@LoadLevel(name = "SEATA", scope = Scope.PROTOTYPE)
public class SeataSerializer implements Serializer {
+ MessageCodecFactory factory;
+ byte protocolVersion ;
+
+ public SeataSerializer(Byte version){
+ if (version == ProtocolConstants.VERSION_1) {
+ factory = new MessageCodecFactoryV1();
+ }else {
+ throw new NotSupportYetException("not support version" + version);
+ }
+ protocolVersion = version;
+ }
@Override
public <T> byte[] serialize(T t) {
if (!(t instanceof AbstractMessage)) {
throw new IllegalArgumentException("AbstractMessage isn't available.");
}
AbstractMessage abstractMessage = (AbstractMessage)t;
- //typecode
+ //type code
short typecode = abstractMessage.getTypeCode();
//msg codec
- MessageSeataCodec messageCodec = MessageCodecFactory.getMessageCodec(typecode);
+ MessageSeataCodec messageCodec = factory.getMessageCodec(typecode);
//get empty ByteBuffer
ByteBuf out = Unpooled.buffer(1024);
//msg encode
messageCodec.encode(t, out);
byte[] body = new byte[out.readableBytes()];
out.readBytes(body);
- //typecode + body
- ByteBuffer byteBuffer = ByteBuffer.allocate(2 + body.length);
- byteBuffer.putShort(typecode);
+ ByteBuffer byteBuffer;
+ if (protocolVersion == ProtocolConstants.VERSION_0) {
Review Comment:
为什么不通过策略模式来实现不同版本的逻辑处理？
Why not implement different versions of logic processing through the policy pattern?
##########
core/src/main/java/io/seata/core/serializer/SerializerServiceLoader.java:
##########
@@ -48,6 +51,24 @@ public static Serializer load(SerializerType type) throws EnhancedServiceNotFoun
"Please manually reference 'io.seata:seata-serializer-protobuf' dependency ", e);
}
}
- return EnhancedServiceLoader.load(Serializer.class, type.name());
+
+ String key = serialzerKey(type, version);
+ Serializer serializer = serializerMap.get(key);
+ if (serializer == null) {
+ if (type == SerializerType.SEATA) {
+ serializer = EnhancedServiceLoader.load(Serializer.class, type.name(), new Object[]{version});
Review Comment:
为何不是通过version加载到对应的实现类，而是共用一个实现类只是在内部对不同版本做不同的处理
Why not load it into the corresponding implementation class through version, but share an implementation class that only handles different versions internally?
--
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: dev-unsubscribe@seata.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@seata.apache.org
For additional commands, e-mail: dev-help@seata.apache.org
Re: [PR] optimize : load SeataSerializer by version [incubator-seata]
Posted by "wangliang181230 (via GitHub)" <gi...@apache.org>.
wangliang181230 commented on code in PR #6208:
URL: https://github.com/apache/incubator-seata/pull/6208#discussion_r1437319755
##########
serializer/seata-serializer-seata/src/main/java/io/seata/serializer/seata/protocol/v0/MessageCodecFactoryV0.java:
##########
@@ -0,0 +1,83 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package io.seata.serializer.seata.protocol.v0;
+
+import io.seata.core.protocol.MessageType;
+import io.seata.serializer.seata.MessageCodecFactory;
+import io.seata.serializer.seata.MessageSeataCodec;
+import io.seata.serializer.seata.protocol.BatchResultMessageCodec;
+import io.seata.serializer.seata.protocol.MergeResultMessageCodec;
+import io.seata.serializer.seata.protocol.MergedWarpMessageCodec;
+import io.seata.serializer.seata.protocol.RegisterRMRequestCodec;
+import io.seata.serializer.seata.protocol.RegisterRMResponseCodec;
+import io.seata.serializer.seata.protocol.RegisterTMRequestCodec;
+import io.seata.serializer.seata.protocol.RegisterTMResponseCodec;
+import io.seata.serializer.seata.protocol.transaction.BranchCommitRequestCodec;
+import io.seata.serializer.seata.protocol.transaction.BranchRollbackRequestCodec;
+import io.seata.serializer.seata.protocol.transaction.GlobalReportRequestCodec;
+
+/**
+ * The type Message codec factory v0.
+ */
+public class MessageCodecFactoryV0 extends MessageCodecFactory {
+
+ /**
+ * Gets msg instance by code.
+ *
+ * @param typeCode the type code
+ * @return the msg instance by code
+ */
+ public MessageSeataCodec getMessageCodec(short typeCode) {
Review Comment:
Missing `@Override`
--
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: dev-unsubscribe@seata.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@seata.apache.org
For additional commands, e-mail: dev-help@seata.apache.org
Re: [PR] optimize : load SeataSerializer by version [incubator-seata]
Posted by "funky-eyes (via GitHub)" <gi...@apache.org>.
funky-eyes commented on code in PR #6208:
URL: https://github.com/apache/incubator-seata/pull/6208#discussion_r1451254322
##########
serializer/seata-serializer-seata/src/main/java/io/seata/serializer/seata/SeataSerializer.java:
##########
@@ -16,52 +16,133 @@
*/
package io.seata.serializer.seata;
-import java.nio.ByteBuffer;
-
import io.netty.buffer.ByteBuf;
import io.netty.buffer.Unpooled;
import io.seata.common.loader.LoadLevel;
+import io.seata.common.loader.Scope;
import io.seata.common.util.BufferUtils;
import io.seata.core.protocol.AbstractMessage;
+import io.seata.core.protocol.ProtocolConstants;
import io.seata.core.serializer.Serializer;
+import java.nio.ByteBuffer;
+
/**
* The Seata codec.
- *
*/
-@LoadLevel(name = "SEATA")
+@LoadLevel(name = "SEATA", scope = Scope.PROTOTYPE)
public class SeataSerializer implements Serializer {
+ Serializer versionSeataSerializer;
+
+ Serializer seataSerializerV0 = new SeataSerializerV0();
+ Serializer seataSerializerV1 = new SeataSerializerV1();
Review Comment:
> version取决于客户端，理论上是都有可能用到的（比如突然有个v1的客户端过来连接），当然也可以改成懒加载的
>
> version depends on the client, theoretically it is possible to use (for example, suddenly there is a v1 client to connect), of course, can also be changed to lazy loading
You've already cached the corresponding version of the seataserializer on your use side, and this is also a multiple instance, so why instantiate 2 of them?
--
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@seata.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@seata.apache.org
For additional commands, e-mail: notifications-help@seata.apache.org
Re: [PR] optimize : load SeataSerializer by version [incubator-seata]
Posted by "wangliang181230 (via GitHub)" <gi...@apache.org>.
wangliang181230 commented on code in PR #6208:
URL: https://github.com/apache/incubator-seata/pull/6208#discussion_r1437321076
##########
serializer/seata-serializer-seata/src/main/java/io/seata/serializer/seata/protocol/v0/MessageCodecFactoryV0.java:
##########
@@ -0,0 +1,83 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package io.seata.serializer.seata.protocol.v0;
+
+import io.seata.core.protocol.MessageType;
+import io.seata.serializer.seata.MessageCodecFactory;
+import io.seata.serializer.seata.MessageSeataCodec;
+import io.seata.serializer.seata.protocol.BatchResultMessageCodec;
+import io.seata.serializer.seata.protocol.MergeResultMessageCodec;
+import io.seata.serializer.seata.protocol.MergedWarpMessageCodec;
+import io.seata.serializer.seata.protocol.RegisterRMRequestCodec;
+import io.seata.serializer.seata.protocol.RegisterRMResponseCodec;
+import io.seata.serializer.seata.protocol.RegisterTMRequestCodec;
+import io.seata.serializer.seata.protocol.RegisterTMResponseCodec;
+import io.seata.serializer.seata.protocol.transaction.BranchCommitRequestCodec;
+import io.seata.serializer.seata.protocol.transaction.BranchRollbackRequestCodec;
+import io.seata.serializer.seata.protocol.transaction.GlobalReportRequestCodec;
+
+/**
+ * The type Message codec factory v0.
+ */
+public class MessageCodecFactoryV0 extends MessageCodecFactory {
+
+ /**
+ * Gets msg instance by code.
+ *
+ * @param typeCode the type code
+ * @return the msg instance by code
+ */
+ public MessageSeataCodec getMessageCodec(short typeCode) {
+ MessageSeataCodec msgCodec = null;
+ switch (typeCode) {
+ case MessageType.TYPE_SEATA_MERGE:
+ msgCodec = new MergedWarpMessageCodec();
+ break;
+ case MessageType.TYPE_SEATA_MERGE_RESULT:
+ msgCodec = new MergeResultMessageCodec();
+ break;
+ case MessageType.TYPE_REG_CLT:
+ msgCodec = new RegisterTMRequestCodec();
+ break;
+ case MessageType.TYPE_REG_CLT_RESULT:
+ msgCodec = new RegisterTMResponseCodec();
+ break;
+ case MessageType.TYPE_REG_RM:
+ msgCodec = new RegisterRMRequestCodec();
+ break;
+ case MessageType.TYPE_REG_RM_RESULT:
+ msgCodec = new RegisterRMResponseCodec();
+ break;
+ case MessageType.TYPE_BRANCH_COMMIT:
+ msgCodec = new BranchCommitRequestCodec();
+ break;
+ case MessageType.TYPE_BRANCH_ROLLBACK:
+ msgCodec = new BranchRollbackRequestCodec();
+ break;
+ case MessageType.TYPE_GLOBAL_REPORT:
+ msgCodec = new GlobalReportRequestCodec();
+ break;
+ case MessageType.TYPE_BATCH_RESULT_MSG:
+ msgCodec = new BatchResultMessageCodec();
+ break;
+ default:
+ break;
+ }
Review Comment:
Encapsulate this code into a method and reuse it.
封装这段代码到一个方法中，复用它。
--
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: dev-unsubscribe@seata.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@seata.apache.org
For additional commands, e-mail: dev-help@seata.apache.org
Re: [PR] optimize : load SeataSerializer by version [incubator-seata]
Posted by "funky-eyes (via GitHub)" <gi...@apache.org>.
funky-eyes commented on code in PR #6208:
URL: https://github.com/apache/incubator-seata/pull/6208#discussion_r1438064621
##########
serializer/seata-serializer-seata/src/main/java/io/seata/serializer/seata/SeataSerializer.java:
##########
@@ -16,72 +16,43 @@
*/
package io.seata.serializer.seata;
-import java.nio.ByteBuffer;
-
-import io.netty.buffer.ByteBuf;
-import io.netty.buffer.Unpooled;
+import com.google.common.collect.ImmutableMap;
import io.seata.common.loader.LoadLevel;
-import io.seata.common.util.BufferUtils;
-import io.seata.core.protocol.AbstractMessage;
+import io.seata.common.loader.Scope;
+import io.seata.core.protocol.ProtocolConstants;
import io.seata.core.serializer.Serializer;
+import io.seata.serializer.seata.protocol.v0.SeataSerializerV0;
+import io.seata.serializer.seata.protocol.v1.SeataSerializerV1;
+
+import java.util.Map;
/**
* The Seata codec.
- *
*/
-@LoadLevel(name = "SEATA")
+@LoadLevel(name = "SEATA", scope = Scope.PROTOTYPE)
public class SeataSerializer implements Serializer {
- @Override
- public <T> byte[] serialize(T t) {
- if (!(t instanceof AbstractMessage)) {
- throw new IllegalArgumentException("AbstractMessage isn't available.");
- }
- AbstractMessage abstractMessage = (AbstractMessage)t;
- //typecode
- short typecode = abstractMessage.getTypeCode();
- //msg codec
- MessageSeataCodec messageCodec = MessageCodecFactory.getMessageCodec(typecode);
- //get empty ByteBuffer
- ByteBuf out = Unpooled.buffer(1024);
- //msg encode
- messageCodec.encode(t, out);
- byte[] body = new byte[out.readableBytes()];
- out.readBytes(body);
+ Serializer versionSeataSerializer;
- //typecode + body
- ByteBuffer byteBuffer = ByteBuffer.allocate(2 + body.length);
- byteBuffer.putShort(typecode);
- byteBuffer.put(body);
+ static Map<Byte, Serializer> VERSION_MAP = ImmutableMap.<Byte, Serializer>builder()
+ .put(ProtocolConstants.VERSION_0, new SeataSerializerV0())
+ .put(ProtocolConstants.VERSION_1, new SeataSerializerV1())
+ .build();
- BufferUtils.flip(byteBuffer);
- byte[] content = new byte[byteBuffer.limit()];
- byteBuffer.get(content);
- return content;
+ public SeataSerializer(Byte version){
+ versionSeataSerializer = VERSION_MAP.get(version);
Review Comment:
为什么需要一个map，我认为使用if else构建对应版本的序列化器即可，因为本身spi加载到时已经对SeataSerializer进行了缓存
Why do you need a map? I think it is enough to use if else to build the corresponding version of the serializer, because the SeataSerializer has already been cached when the spi is loaded.
##########
serializer/seata-serializer-seata/src/main/java/io/seata/serializer/seata/protocol/MergeResultMessageCodec.java:
##########
@@ -24,13 +24,16 @@
import io.seata.core.protocol.AbstractMessage;
import io.seata.core.protocol.AbstractResultMessage;
import io.seata.core.protocol.MergeResultMessage;
+import io.seata.serializer.seata.protocol.v1.MessageCodecFactoryV1;
/**
* The type Merge result message codec.
*
*/
public class MergeResultMessageCodec extends AbstractMessageCodec {
+ protected MessageCodecFactory factory = new MessageCodecFactoryV1();
Review Comment:
为什么MessageCodecFactory不通过spi形式加载，我认为它只需要一个单例然后复用即可
Why doesn't MessageCodecFactory load as spi, I think it just needs a singleton and then reuse it
##########
core/src/main/java/io/seata/core/rpc/netty/v1/ProtocolV1Encoder.java:
##########
@@ -92,7 +92,7 @@ public void encode(ChannelHandlerContext ctx, Object msg, ByteBuf out) {
if (messageType != ProtocolConstants.MSGTYPE_HEARTBEAT_REQUEST
&& messageType != ProtocolConstants.MSGTYPE_HEARTBEAT_RESPONSE) {
// heartbeat has no body
- Serializer serializer = SerializerServiceLoader.load(SerializerType.getByCode(rpcMessage.getCodec()));
+ Serializer serializer = SerializerServiceLoader.load(SerializerType.getByCode(rpcMessage.getCodec()), ProtocolConstants.VERSION_1);
Review Comment:
除了序列化，v0的protocolencoder和decoder在哪？因为v0是没有header这些内容的
Except for serialization, where are the protocolencoder and decoder of v0? Because v0 has no headers
--
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: dev-unsubscribe@seata.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@seata.apache.org
For additional commands, e-mail: dev-help@seata.apache.org
Re: [PR] optimize : load SeataSerializer by version [incubator-seata]
Posted by "funky-eyes (via GitHub)" <gi...@apache.org>.
funky-eyes commented on code in PR #6208:
URL: https://github.com/apache/incubator-seata/pull/6208#discussion_r1591860000
##########
serializer/seata-serializer-seata/src/main/java/org/apache/seata/serializer/seata/SeataSerializer.java:
##########
@@ -16,52 +16,149 @@
*/
package org.apache.seata.serializer.seata;
-import java.nio.ByteBuffer;
-
import io.netty.buffer.ByteBuf;
import io.netty.buffer.Unpooled;
import org.apache.seata.common.loader.LoadLevel;
+import org.apache.seata.common.loader.Scope;
import org.apache.seata.common.util.BufferUtils;
import org.apache.seata.core.protocol.AbstractMessage;
+import org.apache.seata.core.protocol.ProtocolConstants;
import org.apache.seata.core.serializer.Serializer;
+import java.nio.ByteBuffer;
+
/**
* The Seata codec.
- *
*/
-@LoadLevel(name = "SEATA")
+@LoadLevel(name = "SEATA", scope = Scope.PROTOTYPE)
public class SeataSerializer implements Serializer {
+ Serializer versionSeataSerializer;
+
+ public SeataSerializer(Byte version) {
+ if (version == ProtocolConstants.VERSION_0) {
+ versionSeataSerializer = SeataSerializerV0.getInstance();
+ } else if (version == ProtocolConstants.VERSION_1) {
+ versionSeataSerializer = SeataSerializerV1.getInstance();
+ }
+ if (versionSeataSerializer == null) {
+ throw new UnsupportedOperationException("version is not supported");
+ }
+ }
@Override
public <T> byte[] serialize(T t) {
- if (!(t instanceof AbstractMessage)) {
- throw new IllegalArgumentException("AbstractMessage isn't available.");
- }
- AbstractMessage abstractMessage = (AbstractMessage)t;
- //typecode
- short typecode = abstractMessage.getTypeCode();
- //msg codec
- MessageSeataCodec messageCodec = MessageCodecFactory.getMessageCodec(typecode);
- //get empty ByteBuffer
- ByteBuf out = Unpooled.buffer(1024);
- //msg encode
- messageCodec.encode(t, out);
- byte[] body = new byte[out.readableBytes()];
- out.readBytes(body);
-
- //typecode + body
- ByteBuffer byteBuffer = ByteBuffer.allocate(2 + body.length);
- byteBuffer.putShort(typecode);
- byteBuffer.put(body);
-
- BufferUtils.flip(byteBuffer);
- byte[] content = new byte[byteBuffer.limit()];
- byteBuffer.get(content);
- return content;
+ return versionSeataSerializer.serialize(t);
}
@Override
public <T> T deserialize(byte[] bytes) {
+ return versionSeataSerializer.deserialize(bytes);
+ }
+
+
+ static class SeataSerializerV1 implements Serializer {
+
+ private static SeataSerializerV1 instance;
+
+ private SeataSerializerV1() {
+ }
+
+ public static SeataSerializerV1 getInstance() {
+ if (instance == null) {
+ synchronized (SeataSerializerV1.class) {
+ instance = new SeataSerializerV1();
Review Comment:
没有进行双检锁
No double-checked locking.
##########
serializer/seata-serializer-seata/src/main/java/org/apache/seata/serializer/seata/SeataSerializer.java:
##########
@@ -16,52 +16,149 @@
*/
package org.apache.seata.serializer.seata;
-import java.nio.ByteBuffer;
-
import io.netty.buffer.ByteBuf;
import io.netty.buffer.Unpooled;
import org.apache.seata.common.loader.LoadLevel;
+import org.apache.seata.common.loader.Scope;
import org.apache.seata.common.util.BufferUtils;
import org.apache.seata.core.protocol.AbstractMessage;
+import org.apache.seata.core.protocol.ProtocolConstants;
import org.apache.seata.core.serializer.Serializer;
+import java.nio.ByteBuffer;
+
/**
* The Seata codec.
- *
*/
-@LoadLevel(name = "SEATA")
+@LoadLevel(name = "SEATA", scope = Scope.PROTOTYPE)
public class SeataSerializer implements Serializer {
+ Serializer versionSeataSerializer;
+
+ public SeataSerializer(Byte version) {
+ if (version == ProtocolConstants.VERSION_0) {
+ versionSeataSerializer = SeataSerializerV0.getInstance();
+ } else if (version == ProtocolConstants.VERSION_1) {
+ versionSeataSerializer = SeataSerializerV1.getInstance();
+ }
+ if (versionSeataSerializer == null) {
+ throw new UnsupportedOperationException("version is not supported");
+ }
+ }
@Override
public <T> byte[] serialize(T t) {
- if (!(t instanceof AbstractMessage)) {
- throw new IllegalArgumentException("AbstractMessage isn't available.");
- }
- AbstractMessage abstractMessage = (AbstractMessage)t;
- //typecode
- short typecode = abstractMessage.getTypeCode();
- //msg codec
- MessageSeataCodec messageCodec = MessageCodecFactory.getMessageCodec(typecode);
- //get empty ByteBuffer
- ByteBuf out = Unpooled.buffer(1024);
- //msg encode
- messageCodec.encode(t, out);
- byte[] body = new byte[out.readableBytes()];
- out.readBytes(body);
-
- //typecode + body
- ByteBuffer byteBuffer = ByteBuffer.allocate(2 + body.length);
- byteBuffer.putShort(typecode);
- byteBuffer.put(body);
-
- BufferUtils.flip(byteBuffer);
- byte[] content = new byte[byteBuffer.limit()];
- byteBuffer.get(content);
- return content;
+ return versionSeataSerializer.serialize(t);
}
@Override
public <T> T deserialize(byte[] bytes) {
+ return versionSeataSerializer.deserialize(bytes);
+ }
+
+
+ static class SeataSerializerV1 implements Serializer {
+
+ private static SeataSerializerV1 instance;
+
+ private SeataSerializerV1() {
+ }
+
+ public static SeataSerializerV1 getInstance() {
+ if (instance == null) {
+ synchronized (SeataSerializerV1.class) {
+ instance = new SeataSerializerV1();
+ }
+ }
+ return instance;
+ }
+
+ @Override
+ public <T> byte[] serialize(T t) {
+ if (!(t instanceof AbstractMessage)) {
+ throw new IllegalArgumentException("AbstractMessage isn't available.");
+ }
+ AbstractMessage abstractMessage = (AbstractMessage) t;
+ //type code
+ short typecode = abstractMessage.getTypeCode();
+ //msg codec
+ MessageSeataCodec messageCodec = MessageCodecFactory.getMessageCodec(typecode, ProtocolConstants.VERSION_1);
+ //get empty ByteBuffer
+ ByteBuf out = Unpooled.buffer(1024);
+ //msg encode
+ messageCodec.encode(t, out);
+ byte[] body = new byte[out.readableBytes()];
+ out.readBytes(body);
+
+ ByteBuffer byteBuffer;
+
+ //typecode + body
+ byteBuffer = ByteBuffer.allocate(2 + body.length);
+ byteBuffer.putShort(typecode);
+ byteBuffer.put(body);
+
+ BufferUtils.flip(byteBuffer);
+ byte[] content = new byte[byteBuffer.limit()];
+ byteBuffer.get(content);
+ return content;
+ }
+
+ @Override
+ public <T> T deserialize(byte[] bytes) {
+ return deserializeByVersion(bytes, ProtocolConstants.VERSION_1);
+ }
+ }
+ static class SeataSerializerV0 implements Serializer {
+
+ private static SeataSerializerV0 instance;
+
+ private SeataSerializerV0() {
+ }
+
+ public static SeataSerializerV0 getInstance() {
+ if (instance == null) {
+ synchronized (SeataSerializerV0.class) {
+ instance = new SeataSerializerV0();
Review Comment:
No double-checked locking.
--
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@seata.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@seata.apache.org
For additional commands, e-mail: notifications-help@seata.apache.org
Re: [PR] optimize : load SeataSerializer by version [incubator-seata]
Posted by "Bughue (via GitHub)" <gi...@apache.org>.
Bughue commented on code in PR #6208:
URL: https://github.com/apache/incubator-seata/pull/6208#discussion_r1449775984
##########
serializer/seata-serializer-seata/src/main/java/io/seata/serializer/seata/SeataSerializer.java:
##########
@@ -16,52 +16,133 @@
*/
package io.seata.serializer.seata;
-import java.nio.ByteBuffer;
-
import io.netty.buffer.ByteBuf;
import io.netty.buffer.Unpooled;
import io.seata.common.loader.LoadLevel;
+import io.seata.common.loader.Scope;
import io.seata.common.util.BufferUtils;
import io.seata.core.protocol.AbstractMessage;
+import io.seata.core.protocol.ProtocolConstants;
import io.seata.core.serializer.Serializer;
+import java.nio.ByteBuffer;
+
/**
* The Seata codec.
- *
*/
-@LoadLevel(name = "SEATA")
+@LoadLevel(name = "SEATA", scope = Scope.PROTOTYPE)
public class SeataSerializer implements Serializer {
+ Serializer versionSeataSerializer;
+
+ Serializer seataSerializerV0 = new SeataSerializerV0();
+ Serializer seataSerializerV1 = new SeataSerializerV1();
Review Comment:
version取决于客户端，理论上是都有可能用到的（比如突然有个v1的客户端过来连接），当然也可以改成懒加载的
version depends on the client, theoretically it is possible to use (for example, suddenly there is a v1 client to connect), of course, can also be changed to lazy loading
--
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@seata.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@seata.apache.org
For additional commands, e-mail: notifications-help@seata.apache.org
Re: [PR] optimize : load SeataSerializer by version [incubator-seata]
Posted by "Bughue (via GitHub)" <gi...@apache.org>.
Bughue commented on code in PR #6208:
URL: https://github.com/apache/incubator-seata/pull/6208#discussion_r1449775984
##########
serializer/seata-serializer-seata/src/main/java/io/seata/serializer/seata/SeataSerializer.java:
##########
@@ -16,52 +16,133 @@
*/
package io.seata.serializer.seata;
-import java.nio.ByteBuffer;
-
import io.netty.buffer.ByteBuf;
import io.netty.buffer.Unpooled;
import io.seata.common.loader.LoadLevel;
+import io.seata.common.loader.Scope;
import io.seata.common.util.BufferUtils;
import io.seata.core.protocol.AbstractMessage;
+import io.seata.core.protocol.ProtocolConstants;
import io.seata.core.serializer.Serializer;
+import java.nio.ByteBuffer;
+
/**
* The Seata codec.
- *
*/
-@LoadLevel(name = "SEATA")
+@LoadLevel(name = "SEATA", scope = Scope.PROTOTYPE)
public class SeataSerializer implements Serializer {
+ Serializer versionSeataSerializer;
+
+ Serializer seataSerializerV0 = new SeataSerializerV0();
+ Serializer seataSerializerV1 = new SeataSerializerV1();
Review Comment:
version取决于客户端，理论上是都有可能用到的（比如突然有个v1的客户端过来连接），当然也可以改成懒加载的
version depends on the client, theoretically it is possible to use (for example, suddenly there is a v1 client to connect), of course, can also be changed to lazy loading of the
--
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@seata.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@seata.apache.org
For additional commands, e-mail: notifications-help@seata.apache.org
Re: [PR] optimize : load SeataSerializer by version [incubator-seata]
Posted by "funky-eyes (via GitHub)" <gi...@apache.org>.
funky-eyes commented on code in PR #6208:
URL: https://github.com/apache/incubator-seata/pull/6208#discussion_r1440146986
##########
serializer/seata-serializer-seata/src/main/java/io/seata/serializer/seata/protocol/v0/SeataSerializerV0.java:
##########
Review Comment:
请将V0和V1的实现都改用匿名内部类方式的单例模式
Please change the implementation of V0 and V1 to the singleton mode of anonymity inner 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: dev-unsubscribe@seata.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@seata.apache.org
For additional commands, e-mail: dev-help@seata.apache.org
Re: [PR] optimize : load SeataSerializer by version [incubator-seata]
Posted by "funky-eyes (via GitHub)" <gi...@apache.org>.
funky-eyes commented on code in PR #6208:
URL: https://github.com/apache/incubator-seata/pull/6208#discussion_r1442423580
##########
serializer/seata-serializer-seata/src/main/java/io/seata/serializer/seata/SeataSerializer.java:
##########
@@ -16,52 +16,133 @@
*/
package io.seata.serializer.seata;
-import java.nio.ByteBuffer;
-
import io.netty.buffer.ByteBuf;
import io.netty.buffer.Unpooled;
import io.seata.common.loader.LoadLevel;
+import io.seata.common.loader.Scope;
import io.seata.common.util.BufferUtils;
import io.seata.core.protocol.AbstractMessage;
+import io.seata.core.protocol.ProtocolConstants;
import io.seata.core.serializer.Serializer;
+import java.nio.ByteBuffer;
+
/**
* The Seata codec.
- *
*/
-@LoadLevel(name = "SEATA")
+@LoadLevel(name = "SEATA", scope = Scope.PROTOTYPE)
Review Comment:
为什么它是一个多例？
Why is it a PROTOTYPE case?
##########
serializer/seata-serializer-seata/src/main/java/io/seata/serializer/seata/SeataSerializer.java:
##########
@@ -16,52 +16,133 @@
*/
package io.seata.serializer.seata;
-import java.nio.ByteBuffer;
-
import io.netty.buffer.ByteBuf;
import io.netty.buffer.Unpooled;
import io.seata.common.loader.LoadLevel;
+import io.seata.common.loader.Scope;
import io.seata.common.util.BufferUtils;
import io.seata.core.protocol.AbstractMessage;
+import io.seata.core.protocol.ProtocolConstants;
import io.seata.core.serializer.Serializer;
+import java.nio.ByteBuffer;
+
/**
* The Seata codec.
- *
*/
-@LoadLevel(name = "SEATA")
+@LoadLevel(name = "SEATA", scope = Scope.PROTOTYPE)
public class SeataSerializer implements Serializer {
+ Serializer versionSeataSerializer;
+
+ Serializer seataSerializerV0 = new SeataSerializerV0();
+ Serializer seataSerializerV1 = new SeataSerializerV1();
+
+ public SeataSerializer(Byte version) {
+ if (version == ProtocolConstants.VERSION_0) {
+ versionSeataSerializer = seataSerializerV0;
+ } else if (version == ProtocolConstants.VERSION_1) {
+ versionSeataSerializer = seataSerializerV1;
+ }
+ if (versionSeataSerializer == null) {
+ throw new UnsupportedOperationException("version is not supported");
+ }
+ }
+
@Override
public <T> byte[] serialize(T t) {
- if (!(t instanceof AbstractMessage)) {
- throw new IllegalArgumentException("AbstractMessage isn't available.");
- }
- AbstractMessage abstractMessage = (AbstractMessage)t;
- //typecode
- short typecode = abstractMessage.getTypeCode();
- //msg codec
- MessageSeataCodec messageCodec = MessageCodecFactory.getMessageCodec(typecode);
- //get empty ByteBuffer
- ByteBuf out = Unpooled.buffer(1024);
- //msg encode
- messageCodec.encode(t, out);
- byte[] body = new byte[out.readableBytes()];
- out.readBytes(body);
-
- //typecode + body
- ByteBuffer byteBuffer = ByteBuffer.allocate(2 + body.length);
- byteBuffer.putShort(typecode);
- byteBuffer.put(body);
-
- BufferUtils.flip(byteBuffer);
- byte[] content = new byte[byteBuffer.limit()];
- byteBuffer.get(content);
- return content;
+ return versionSeataSerializer.serialize(t);
}
@Override
public <T> T deserialize(byte[] bytes) {
+ return versionSeataSerializer.deserialize(bytes);
+ }
+
+
+ static class SeataSerializerV1 implements Serializer {
+
+ private SeataSerializerV1() {
+ }
+
+ @Override
+ public <T> byte[] serialize(T t) {
+ if (!(t instanceof AbstractMessage)) {
+ throw new IllegalArgumentException("AbstractMessage isn't available.");
+ }
+ AbstractMessage abstractMessage = (AbstractMessage) t;
+ //type code
+ short typecode = abstractMessage.getTypeCode();
+ //msg codec
+ MessageSeataCodec messageCodec = MessageCodecFactory.getMessageCodec(typecode, ProtocolConstants.VERSION_1);
+ //get empty ByteBuffer
+ ByteBuf out = Unpooled.buffer(1024);
+ //msg encode
+ messageCodec.encode(t, out);
+ byte[] body = new byte[out.readableBytes()];
+ out.readBytes(body);
+
+ ByteBuffer byteBuffer;
+
+ //typecode + body
+ byteBuffer = ByteBuffer.allocate(2 + body.length);
+ byteBuffer.putShort(typecode);
+ byteBuffer.put(body);
+
+ BufferUtils.flip(byteBuffer);
+ byte[] content = new byte[byteBuffer.limit()];
+ byteBuffer.get(content);
+ return content;
+ }
+
+ @Override
+ public <T> T deserialize(byte[] bytes) {
+ return deserializeByVersion(bytes, ProtocolConstants.VERSION_1);
+
+ }
+ }
+ static class SeataSerializerV0 implements Serializer {
Review Comment:
是做成匿名内部类的单例模式，不是将类放到seataserializer中做内部类
It is a singleton mode to make the anonymity inner class, not to put the class in the seataserializer as the inner 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: dev-unsubscribe@seata.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@seata.apache.org
For additional commands, e-mail: dev-help@seata.apache.org