Skip to content

baidu_std support checksum #2967

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

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

yanglimingcn
Copy link
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
Contributor Author

yanglimingcn commented May 13, 2025

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

@yanglimingcn
Copy link
Contributor Author

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

@yanglimingcn yanglimingcn changed the title support checksum baidu_std support checksum May 13, 2025
@wwbmmm
Copy link
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
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
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
@yanglimingcn yanglimingcn force-pushed the feature/support_checksum branch from 120d549 to f91cc2b Compare June 5, 2025 06:50
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