diff --git a/.github/workflows/codecov.yml b/.github/workflows/codecov.yml index d4c2d688..39c530d6 100644 --- a/.github/workflows/codecov.yml +++ b/.github/workflows/codecov.yml @@ -285,6 +285,9 @@ jobs: - name: Run dnsmasq run: | sudo dnsmasq --local-service -C ${{ github.workspace }}/scripts/coverage/g3bench/dnsmasq.conf + - name: Install thrift compiler + run: | + sudo apt-get install thrift-compiler - name: run unit test run: | ./scripts/coverage/g3bench.sh diff --git a/g3bench/src/target/thrift/protocol/compact/builder.rs b/g3bench/src/target/thrift/protocol/compact/builder.rs index 30057392..788c952f 100644 --- a/g3bench/src/target/thrift/protocol/compact/builder.rs +++ b/g3bench/src/target/thrift/protocol/compact/builder.rs @@ -14,12 +14,10 @@ pub(crate) struct CompactMessageBuilder { impl CompactMessageBuilder { pub(crate) fn new_call(name: &str) -> anyhow::Result { - // the name length is encoded from an unsigned integer, which is different from - // https://github.com/apache/thrift/blob/master/doc/specs/thrift-compact-protocol.md - let name_len = i32::try_from(name.len()).map_err(|_| anyhow!("too long method name"))?; + let name_len = u32::try_from(name.len()).map_err(|_| anyhow!("too long method name"))?; let mut encoder = ThriftVarIntEncoder::default(); - let name_len_bytes = encoder.encode_i32(name_len).to_vec(); + let name_len_bytes = encoder.encode_positive_i32(name_len).to_vec(); Ok(CompactMessageBuilder { name: name.to_string(), @@ -43,7 +41,7 @@ impl CompactMessageBuilder { buf.extend_from_slice(&[0x82, 0x21]); let mut encoder = ThriftVarIntEncoder::default(); - let seq_id_bytes = encoder.encode_i32(seq_id); + let seq_id_bytes = encoder.encode_positive_i32(seq_id as u32); buf.extend_from_slice(seq_id_bytes); buf.extend_from_slice(&self.name_len_bytes); @@ -73,11 +71,11 @@ mod tests { fn build_call() { let builder = CompactMessageBuilder::new_call("ping").unwrap(); let mut buf = Vec::new(); - builder.build_call(-1, true, &[0x00], &mut buf).unwrap(); + builder.build_call(1, true, &[0x00], &mut buf).unwrap(); assert_eq!( &buf, &[ - 0x0, 0x0, 0x0, 0x9, 0x82, 0x21, 0x1, 0x8, 0x70, 0x69, 0x6e, 0x67, 0x0 + 0x0, 0x0, 0x0, 0x9, 0x82, 0x21, 0x1, 0x4, 0x70, 0x69, 0x6e, 0x67, 0x0 ] ); } diff --git a/g3bench/src/target/thrift/protocol/compact/parser.rs b/g3bench/src/target/thrift/protocol/compact/parser.rs index bac25d2d..c4ee8634 100644 --- a/g3bench/src/target/thrift/protocol/compact/parser.rs +++ b/g3bench/src/target/thrift/protocol/compact/parser.rs @@ -48,8 +48,7 @@ impl CompactMessageParser { })?; let left = &left[name_len.encoded_len()..]; - let name_len = usize::try_from(name_len.value()) - .map_err(|_| ThriftResponseMessageParseError::InvalidNameLength)?; + let name_len = name_len.positive_value() as usize; if left.len() < name_len { return Err(ThriftResponseMessageParseError::NoEnoughData); } @@ -60,7 +59,7 @@ impl CompactMessageParser { Ok(ThriftResponseMessage { method: name.to_string(), - seq_id: seq_id.value(), + seq_id: seq_id.positive_value() as i32, encoded_length: data.len(), }) } diff --git a/g3bench/src/target/thrift/tcp/header/thrift/builder.rs b/g3bench/src/target/thrift/tcp/header/thrift/builder.rs index 421a7792..a854efa3 100644 --- a/g3bench/src/target/thrift/tcp/header/thrift/builder.rs +++ b/g3bench/src/target/thrift/tcp/header/thrift/builder.rs @@ -23,7 +23,7 @@ impl TryFrom for StringValue { type Error = anyhow::Error; fn try_from(value: String) -> Result { - let len = i32::try_from(value.len()).map_err(|_| { + let len = u32::try_from(value.len()).map_err(|_| { anyhow!( "too long Thrift THeader string value length {}", value.len() @@ -31,7 +31,7 @@ impl TryFrom for StringValue { })?; let mut encoder = ThriftVarIntEncoder::default(); Ok(StringValue { - len_bytes: encoder.encode_i32(len).to_vec(), + len_bytes: encoder.encode_positive_i32(len).to_vec(), value, }) } diff --git a/lib/g3-types/src/codec/thrift/var_int.rs b/lib/g3-types/src/codec/thrift/var_int.rs index 1e7f6097..c62bea37 100644 --- a/lib/g3-types/src/codec/thrift/var_int.rs +++ b/lib/g3-types/src/codec/thrift/var_int.rs @@ -17,6 +17,10 @@ impl ThriftVarInt32 { Ok(ThriftVarInt32 { leb128 }) } + pub fn positive_value(&self) -> u32 { + self.leb128.value() + } + pub fn value(&self) -> i32 { let uv = self.leb128.value(); i32::from_zig_zag(uv) @@ -37,4 +41,8 @@ impl ThriftVarIntEncoder { let uv = v.to_zig_zag(); self.leb128.encode_u32(uv) } + + pub fn encode_positive_i32(&mut self, v: u32) -> &[u8] { + self.leb128.encode_u32(v) + } } diff --git a/scripts/coverage/g3bench/target_thrift_tcp.sh b/scripts/coverage/g3bench/target_thrift_tcp.sh index 56e693eb..ea78d37c 100644 --- a/scripts/coverage/g3bench/target_thrift_tcp.sh +++ b/scripts/coverage/g3bench/target_thrift_tcp.sh @@ -28,3 +28,63 @@ g3bench thrift tcp --target 127.0.0.1:8888 --check-message-length 22 --binary -- g3bench thrift tcp --target 127.0.0.1:8888 --check-message-length 22 --binary --kitex-ttheader --info-int-kv "4:not-default" echo ${KITEX_REQUEST} kill -INT $KITEX_PID + +# Thrift go tutorial + +git clone https://github.com/apache/thrift.git --depth 1 + +cd thrift/tutorial/go + +thrift --gen go:thrift_import=github.com/apache/thrift/lib/go/thrift,package_prefix=github.com/apache/thrift/tutorial/go/gen-go/ ../shared.thrift +thrift --gen go:thrift_import=github.com/apache/thrift/lib/go/thrift,package_prefix=github.com/apache/thrift/tutorial/go/gen-go/ ../tutorial.thrift + +go build -o go-tutorial ./src/*.go +cd - + +## Binary Framed + +./thrift/tutorial/go/go-tutorial -server -addr 127.0.0.1:9090 -P binary --framed & +THRIFT_GO_PID=$! + +sleep 1 + +g3bench thrift tcp --target 127.0.0.1:9090 --check-message-length 1 --binary --framed ping 00 +g3bench thrift tcp --target 127.0.0.1:9090 --check-message-length 8 --binary --framed add "080001000000010800020000000100" + +kill -INT $THRIFT_GO_PID + +## Binary + +./thrift/tutorial/go/go-tutorial -server -addr 127.0.0.1:9091 -P binary --buffered & +THRIFT_GO_PID=$! + +sleep 1 + +g3bench thrift tcp --target 127.0.0.1:9091 --check-message-length 1 --binary ping 00 +g3bench thrift tcp --target 127.0.0.1:9091 --check-message-length 8 --binary add "080001000000010800020000000100" + +kill -INT $THRIFT_GO_PID + +## Compact Framed + +./thrift/tutorial/go/go-tutorial -server -addr 127.0.0.1:9092 -P compact --framed & +THRIFT_GO_PID=$! + +sleep 1 + +g3bench thrift tcp --target 127.0.0.1:9092 --check-message-length 1 --compact --framed ping 00 +g3bench thrift tcp --target 127.0.0.1:9092 --check-message-length 4 --compact --framed add "1502150200" + +kill -INT $THRIFT_GO_PID + +## Compact + +./thrift/tutorial/go/go-tutorial -server -addr 127.0.0.1:9093 -P compact --buffered & +THRIFT_GO_PID=$! + +sleep 1 + +g3bench thrift tcp --target 127.0.0.1:9093 --check-message-length 1 --compact ping 00 +g3bench thrift tcp --target 127.0.0.1:9093 --check-message-length 4 --compact add "1502150200" + +kill -INT $THRIFT_GO_PID