Skip to content

baidu_std support checksum#2967

Merged
wwbmmm merged 1 commit intoapache:masterfrom
yanglimingcn:feature/support_checksum
Jun 20, 2025
Merged

baidu_std support checksum#2967
wwbmmm merged 1 commit intoapache:masterfrom
yanglimingcn:feature/support_checksum

Conversation

@yanglimingcn
Copy link
Copy Markdown
Contributor

What problem does this PR solve?

Issue Number: #2159

Problem Summary:

Implementing Checksum Functionality at the bRPC Level:

General Requirement: Similar to the compression feature, implement a unified solution at the bRPC level.
Better Performance: Implementing checksum at the user level requires users to serialize protobuf or other data packets and then calculate the checksum. Implementing it at the bRPC level allows direct use of the serialized results for checksum calculation.
Better Compatibility: Implementing checksum at the user level means users work with protobuf data packets. If the protocol changes and the client and server protocols become inconsistent, checksum calculations based on protobuf will fail, preventing users from leveraging protobuf's compatibility features.

What is changed and the side effects?

Changed:

Side effects:

  • Performance effects:

  • Breaking backward compatibility:


Check List:

@yanglimingcn yanglimingcn force-pushed the feature/support_checksum branch 3 times, most recently from e5d953e to e5793e4 Compare May 12, 2025 09:40
@yanglimingcn
Copy link
Copy Markdown
Contributor Author

yanglimingcn commented May 13, 2025

image 这个PR中协议的checksum_value字段目前定义为uint32,可能后续扩展性不太好,大家有什么建议。

@yanglimingcn
Copy link
Copy Markdown
Contributor Author

@wwbmmm @chenBright 辛苦看看,给点建议。

@yanglimingcn yanglimingcn changed the title support checksum baidu_std support checksum May 13, 2025
@wwbmmm
Copy link
Copy Markdown
Contributor

wwbmmm commented May 20, 2025

这个PR中协议的checksum_value字段目前定义为uint32,可能后续扩展性不太好,大家有什么建议。

maybe define checksum_value as bytes in proto, as std::string in code?

@yanglimingcn yanglimingcn force-pushed the feature/support_checksum branch from e5793e4 to 3b3196b Compare May 20, 2025 12:14
@yanglimingcn
Copy link
Copy Markdown
Contributor Author

这个PR中协议的checksum_value字段目前定义为uint32,可能后续扩展性不太好,大家有什么建议。

maybe define checksum_value as bytes in proto, as std::string in code?

done

@yanglimingcn yanglimingcn force-pushed the feature/support_checksum branch from 3b3196b to d80ce4f Compare May 21, 2025 09:11
@wwbmmm
Copy link
Copy Markdown
Contributor

wwbmmm commented May 21, 2025

LGTM

@yanglimingcn yanglimingcn force-pushed the feature/support_checksum branch from d80ce4f to 120d549 Compare May 22, 2025 06:31
Comment thread src/brpc/policy/crc32c_checksum.cpp
Comment thread src/brpc/checksum.h Outdated
@yanglimingcn yanglimingcn force-pushed the feature/support_checksum branch from 120d549 to f91cc2b Compare June 5, 2025 06:50
Comment thread src/brpc/checksum.h Outdated
@yanglimingcn yanglimingcn force-pushed the feature/support_checksum branch from f91cc2b to c541f82 Compare June 9, 2025 01:14
Copy link
Copy Markdown
Contributor

@chenBright chenBright left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@wwbmmm wwbmmm merged commit 663625a into apache:master Jun 20, 2025
15 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants