You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@pegasus.apache.org by "GehaFearless (via GitHub)" <gi...@apache.org> on 2023/05/29 06:28:14 UTC
[GitHub] [incubator-pegasus] GehaFearless opened a new pull request, #1496: feat(FQDN): serialization format rpc_host_port
GehaFearless opened a new pull request, #1496:
URL: https://github.com/apache/incubator-pegasus/pull/1496
issue: https://github.com/apache/incubator-pegasus/issues/1495
Implement thrift protocol functon read and write for class rpc_host_port.
Both about binary protocol and json.
--
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@pegasus.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@pegasus.apache.org
For additional commands, e-mail: dev-help@pegasus.apache.org
[GitHub] [incubator-pegasus] acelyc111 commented on pull request #1496: feat(FQDN): serialization format rpc_host_port
Posted by "acelyc111 (via GitHub)" <gi...@apache.org>.
acelyc111 commented on PR #1496:
URL: https://github.com/apache/incubator-pegasus/pull/1496#issuecomment-1567298080
@GehaFearless Good! Could you please add some tests to verify that serialize and deserialize both json and binary format is working?
--
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@pegasus.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@pegasus.apache.org
For additional commands, e-mail: dev-help@pegasus.apache.org
[GitHub] [incubator-pegasus] acelyc111 merged pull request #1496: feat(FQDN): serialization format rpc_host_port
Posted by "acelyc111 (via GitHub)" <gi...@apache.org>.
acelyc111 merged PR #1496:
URL: https://github.com/apache/incubator-pegasus/pull/1496
--
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@pegasus.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@pegasus.apache.org
For additional commands, e-mail: dev-help@pegasus.apache.org
[GitHub] [incubator-pegasus] acelyc111 commented on a diff in pull request #1496: feat(FQDN): serialization format rpc_host_port
Posted by "acelyc111 (via GitHub)" <gi...@apache.org>.
acelyc111 commented on code in PR #1496:
URL: https://github.com/apache/incubator-pegasus/pull/1496#discussion_r1222514338
##########
src/runtime/test/host_port_test.cpp:
##########
@@ -202,4 +211,29 @@ TEST(host_port_test, dns_resolver)
}
}
+void send_and_check_host_port_by_different_serialize(dsn_msg_serialize_format t)
+{
+ host_port hp = host_port("localhost", 8080);
+ auto hp_str = hp.to_string();
+ ::dsn::rpc_address server("localhost", 20101);
+
+ dsn::message_ptr mesg_ptr = dsn::message_ex::create_request(RPC_TEST_THRIFT_HOST_PORT_PARSER);
+ mesg_ptr->header->context.u.serialize_format = t;
+
+ ::dsn::marshall(mesg_ptr.get(), hp);
+
+ dsn::task_tracker _tracker;
Review Comment:
rename `_tracker` to `tracker`.
##########
src/runtime/test/host_port_test.cpp:
##########
@@ -202,4 +211,29 @@ TEST(host_port_test, dns_resolver)
}
}
+void send_and_check_host_port_by_different_serialize(dsn_msg_serialize_format t)
+{
+ host_port hp = host_port("localhost", 8080);
+ auto hp_str = hp.to_string();
+ ::dsn::rpc_address server("localhost", 20101);
+
+ dsn::message_ptr mesg_ptr = dsn::message_ex::create_request(RPC_TEST_THRIFT_HOST_PORT_PARSER);
Review Comment:
how about msg_ptr? it's a common practice to shorten "message" as "msg".
##########
src/common/serialization_helper/thrift_helper.h:
##########
@@ -280,6 +281,105 @@ inline uint32_t rpc_address::write(apache::thrift::protocol::TProtocol *oprot) c
}
}
+inline uint32_t host_port::read(apache::thrift::protocol::TProtocol *iprot)
+{
+ std::string host;
+ int16_t port;
+ int8_t type_enum_number;
+
+ uint32_t xfer = 0;
+ auto binary_proto = dynamic_cast<apache::thrift::protocol::TBinaryProtocol *>(iprot);
+ if (binary_proto != nullptr) {
+ // the protocol is binary protocol
+ xfer += iprot->readString(host);
+ xfer += iprot->readI16(port);
+ xfer += iprot->readByte(type_enum_number);
+ } else {
+ // the protocol is json protocol
+ std::string fname;
+ xfer += iprot->readStructBegin(fname);
+
+ int16_t fid;
+ ::apache::thrift::protocol::TType ftype;
+ while (true) {
+ xfer += iprot->readFieldBegin(fname, ftype, fid);
+ if (ftype == ::apache::thrift::protocol::T_STOP) {
+ break;
+ }
+ switch (fid) {
+ case 1:
+ if (ftype == ::apache::thrift::protocol::T_STRING) {
+ xfer += iprot->readString(host);
+ } else {
+ xfer += iprot->skip(ftype);
+ }
+ break;
+ case 2:
+ if (ftype == ::apache::thrift::protocol::T_I16) {
+ xfer += iprot->readI16(port);
+ } else {
+ xfer += iprot->skip(ftype);
+ }
+ break;
+ case 3:
+ if (ftype == ::apache::thrift::protocol::T_BYTE) {
+ xfer += iprot->readByte(type_enum_number);
+ } else {
+ xfer += iprot->skip(ftype);
+ }
+ break;
+ default:
+ xfer += iprot->skip(ftype);
+ break;
+ }
+ xfer += iprot->readFieldEnd();
+ }
+ xfer += iprot->readStructEnd();
+ }
+
+ _host = host;
+ _port = static_cast<uint16_t>(port);
+ _type = static_cast<dsn_host_type_t>(type_enum_number);
+ CHECK(_type == HOST_TYPE_INVALID || _type == HOST_TYPE_IPV4,
+ "only invalid or ipv4 can be serialized.");
+
+ return xfer;
+}
+
+inline uint32_t host_port::write(apache::thrift::protocol::TProtocol *oprot) const
+{
+ CHECK(_type == HOST_TYPE_INVALID || _type == HOST_TYPE_IPV4,
+ "only invalid or ipv4 can be serialized.");
+ uint32_t xfer = 0;
+ auto binary_proto = dynamic_cast<apache::thrift::protocol::TBinaryProtocol *>(oprot);
+ if (binary_proto != nullptr) {
+ // the protocol is binary protocol
+ xfer += oprot->writeString(_host);
+ xfer += oprot->writeI16(static_cast<int16_t>(_port));
+ xfer += oprot->writeByte(static_cast<int8_t>(_type));
+ } else {
+ // the protocol is json protocol
+ xfer += oprot->writeStructBegin("host_port");
+
+ xfer += oprot->writeFieldBegin("_host", ::apache::thrift::protocol::T_STRING, 1);
+ xfer += oprot->writeString(_host);
+ xfer += oprot->writeFieldEnd();
+
+ xfer += oprot->writeFieldBegin("_port", ::apache::thrift::protocol::T_I16, 2);
+ xfer += oprot->writeI16(static_cast<int16_t>(_port));
+ xfer += oprot->writeFieldEnd();
+
+ xfer += oprot->writeFieldBegin("_type", ::apache::thrift::protocol::T_BYTE, 3);
Review Comment:
```suggestion
xfer += oprot->writeFieldBegin("type", ::apache::thrift::protocol::T_BYTE, 3);
```
##########
src/common/serialization_helper/thrift_helper.h:
##########
@@ -280,6 +281,105 @@ inline uint32_t rpc_address::write(apache::thrift::protocol::TProtocol *oprot) c
}
}
+inline uint32_t host_port::read(apache::thrift::protocol::TProtocol *iprot)
+{
+ std::string host;
+ int16_t port;
+ int8_t type_enum_number;
+
+ uint32_t xfer = 0;
+ auto binary_proto = dynamic_cast<apache::thrift::protocol::TBinaryProtocol *>(iprot);
+ if (binary_proto != nullptr) {
+ // the protocol is binary protocol
+ xfer += iprot->readString(host);
+ xfer += iprot->readI16(port);
+ xfer += iprot->readByte(type_enum_number);
+ } else {
+ // the protocol is json protocol
+ std::string fname;
+ xfer += iprot->readStructBegin(fname);
+
+ int16_t fid;
+ ::apache::thrift::protocol::TType ftype;
+ while (true) {
+ xfer += iprot->readFieldBegin(fname, ftype, fid);
+ if (ftype == ::apache::thrift::protocol::T_STOP) {
+ break;
+ }
+ switch (fid) {
+ case 1:
+ if (ftype == ::apache::thrift::protocol::T_STRING) {
+ xfer += iprot->readString(host);
+ } else {
+ xfer += iprot->skip(ftype);
+ }
+ break;
+ case 2:
+ if (ftype == ::apache::thrift::protocol::T_I16) {
+ xfer += iprot->readI16(port);
+ } else {
+ xfer += iprot->skip(ftype);
+ }
+ break;
+ case 3:
+ if (ftype == ::apache::thrift::protocol::T_BYTE) {
+ xfer += iprot->readByte(type_enum_number);
+ } else {
+ xfer += iprot->skip(ftype);
+ }
+ break;
+ default:
+ xfer += iprot->skip(ftype);
+ break;
+ }
+ xfer += iprot->readFieldEnd();
+ }
+ xfer += iprot->readStructEnd();
+ }
+
+ _host = host;
+ _port = static_cast<uint16_t>(port);
+ _type = static_cast<dsn_host_type_t>(type_enum_number);
+ CHECK(_type == HOST_TYPE_INVALID || _type == HOST_TYPE_IPV4,
+ "only invalid or ipv4 can be serialized.");
+
+ return xfer;
+}
+
+inline uint32_t host_port::write(apache::thrift::protocol::TProtocol *oprot) const
+{
+ CHECK(_type == HOST_TYPE_INVALID || _type == HOST_TYPE_IPV4,
+ "only invalid or ipv4 can be serialized.");
+ uint32_t xfer = 0;
+ auto binary_proto = dynamic_cast<apache::thrift::protocol::TBinaryProtocol *>(oprot);
+ if (binary_proto != nullptr) {
+ // the protocol is binary protocol
+ xfer += oprot->writeString(_host);
+ xfer += oprot->writeI16(static_cast<int16_t>(_port));
+ xfer += oprot->writeByte(static_cast<int8_t>(_type));
+ } else {
+ // the protocol is json protocol
+ xfer += oprot->writeStructBegin("host_port");
+
+ xfer += oprot->writeFieldBegin("_host", ::apache::thrift::protocol::T_STRING, 1);
+ xfer += oprot->writeString(_host);
+ xfer += oprot->writeFieldEnd();
+
+ xfer += oprot->writeFieldBegin("_port", ::apache::thrift::protocol::T_I16, 2);
Review Comment:
```suggestion
xfer += oprot->writeFieldBegin("port", ::apache::thrift::protocol::T_I16, 2);
```
##########
src/common/serialization_helper/thrift_helper.h:
##########
@@ -280,6 +281,105 @@ inline uint32_t rpc_address::write(apache::thrift::protocol::TProtocol *oprot) c
}
}
+inline uint32_t host_port::read(apache::thrift::protocol::TProtocol *iprot)
+{
+ std::string host;
+ int16_t port;
+ int8_t type_enum_number;
+
+ uint32_t xfer = 0;
+ auto binary_proto = dynamic_cast<apache::thrift::protocol::TBinaryProtocol *>(iprot);
+ if (binary_proto != nullptr) {
+ // the protocol is binary protocol
+ xfer += iprot->readString(host);
+ xfer += iprot->readI16(port);
+ xfer += iprot->readByte(type_enum_number);
+ } else {
+ // the protocol is json protocol
+ std::string fname;
+ xfer += iprot->readStructBegin(fname);
+
+ int16_t fid;
+ ::apache::thrift::protocol::TType ftype;
+ while (true) {
+ xfer += iprot->readFieldBegin(fname, ftype, fid);
+ if (ftype == ::apache::thrift::protocol::T_STOP) {
+ break;
+ }
+ switch (fid) {
+ case 1:
+ if (ftype == ::apache::thrift::protocol::T_STRING) {
+ xfer += iprot->readString(host);
+ } else {
+ xfer += iprot->skip(ftype);
+ }
+ break;
+ case 2:
+ if (ftype == ::apache::thrift::protocol::T_I16) {
+ xfer += iprot->readI16(port);
+ } else {
+ xfer += iprot->skip(ftype);
+ }
+ break;
+ case 3:
+ if (ftype == ::apache::thrift::protocol::T_BYTE) {
+ xfer += iprot->readByte(type_enum_number);
+ } else {
+ xfer += iprot->skip(ftype);
+ }
+ break;
+ default:
+ xfer += iprot->skip(ftype);
+ break;
+ }
+ xfer += iprot->readFieldEnd();
+ }
+ xfer += iprot->readStructEnd();
+ }
+
+ _host = host;
+ _port = static_cast<uint16_t>(port);
+ _type = static_cast<dsn_host_type_t>(type_enum_number);
+ CHECK(_type == HOST_TYPE_INVALID || _type == HOST_TYPE_IPV4,
+ "only invalid or ipv4 can be serialized.");
Review Comment:
```suggestion
"only invalid or ipv4 can be deserialized.");
```
##########
src/runtime/test/host_port_test.cpp:
##########
@@ -202,4 +211,29 @@ TEST(host_port_test, dns_resolver)
}
}
+void send_and_check_host_port_by_different_serialize(dsn_msg_serialize_format t)
+{
+ host_port hp = host_port("localhost", 8080);
Review Comment:
How about make `hp` as a parameter, and then add more tests?
##########
src/common/serialization_helper/thrift_helper.h:
##########
@@ -280,6 +281,105 @@ inline uint32_t rpc_address::write(apache::thrift::protocol::TProtocol *oprot) c
}
}
+inline uint32_t host_port::read(apache::thrift::protocol::TProtocol *iprot)
+{
+ std::string host;
+ int16_t port;
+ int8_t type_enum_number;
+
+ uint32_t xfer = 0;
+ auto binary_proto = dynamic_cast<apache::thrift::protocol::TBinaryProtocol *>(iprot);
+ if (binary_proto != nullptr) {
+ // the protocol is binary protocol
+ xfer += iprot->readString(host);
+ xfer += iprot->readI16(port);
+ xfer += iprot->readByte(type_enum_number);
+ } else {
+ // the protocol is json protocol
+ std::string fname;
+ xfer += iprot->readStructBegin(fname);
+
+ int16_t fid;
+ ::apache::thrift::protocol::TType ftype;
+ while (true) {
+ xfer += iprot->readFieldBegin(fname, ftype, fid);
+ if (ftype == ::apache::thrift::protocol::T_STOP) {
+ break;
+ }
+ switch (fid) {
+ case 1:
+ if (ftype == ::apache::thrift::protocol::T_STRING) {
+ xfer += iprot->readString(host);
+ } else {
+ xfer += iprot->skip(ftype);
+ }
+ break;
+ case 2:
+ if (ftype == ::apache::thrift::protocol::T_I16) {
+ xfer += iprot->readI16(port);
+ } else {
+ xfer += iprot->skip(ftype);
+ }
+ break;
+ case 3:
+ if (ftype == ::apache::thrift::protocol::T_BYTE) {
+ xfer += iprot->readByte(type_enum_number);
+ } else {
+ xfer += iprot->skip(ftype);
+ }
+ break;
+ default:
+ xfer += iprot->skip(ftype);
+ break;
+ }
+ xfer += iprot->readFieldEnd();
+ }
+ xfer += iprot->readStructEnd();
+ }
+
+ _host = host;
+ _port = static_cast<uint16_t>(port);
+ _type = static_cast<dsn_host_type_t>(type_enum_number);
+ CHECK(_type == HOST_TYPE_INVALID || _type == HOST_TYPE_IPV4,
+ "only invalid or ipv4 can be serialized.");
+
+ return xfer;
+}
+
+inline uint32_t host_port::write(apache::thrift::protocol::TProtocol *oprot) const
+{
+ CHECK(_type == HOST_TYPE_INVALID || _type == HOST_TYPE_IPV4,
+ "only invalid or ipv4 can be serialized.");
+ uint32_t xfer = 0;
+ auto binary_proto = dynamic_cast<apache::thrift::protocol::TBinaryProtocol *>(oprot);
+ if (binary_proto != nullptr) {
+ // the protocol is binary protocol
+ xfer += oprot->writeString(_host);
+ xfer += oprot->writeI16(static_cast<int16_t>(_port));
+ xfer += oprot->writeByte(static_cast<int8_t>(_type));
+ } else {
+ // the protocol is json protocol
+ xfer += oprot->writeStructBegin("host_port");
+
+ xfer += oprot->writeFieldBegin("_host", ::apache::thrift::protocol::T_STRING, 1);
Review Comment:
```suggestion
xfer += oprot->writeFieldBegin("host", ::apache::thrift::protocol::T_STRING, 1);
```
##########
src/runtime/test/host_port_test.cpp:
##########
@@ -202,4 +211,29 @@ TEST(host_port_test, dns_resolver)
}
}
+void send_and_check_host_port_by_different_serialize(dsn_msg_serialize_format t)
+{
+ host_port hp = host_port("localhost", 8080);
+ auto hp_str = hp.to_string();
+ ::dsn::rpc_address server("localhost", 20101);
+
+ dsn::message_ptr mesg_ptr = dsn::message_ex::create_request(RPC_TEST_THRIFT_HOST_PORT_PARSER);
+ mesg_ptr->header->context.u.serialize_format = t;
+
+ ::dsn::marshall(mesg_ptr.get(), hp);
+
+ dsn::task_tracker _tracker;
+ rpc::call(server, mesg_ptr.get(), &_tracker, [hp_str](error_code ec, std::string &&resp) {
+ if (ERR_OK == ec) {
Review Comment:
Is it always ERR_OK? add ASSERT_EQ(ERR_OK, ec) if it is.
--
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@pegasus.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@pegasus.apache.org
For additional commands, e-mail: dev-help@pegasus.apache.org