Skip to content
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

[feature request] C++ and Python support include message header size when check maxMessageSize. #17188

Closed
1 of 2 tasks
shibd opened this issue Aug 19, 2022 · 2 comments
Closed
1 of 2 tasks
Labels

Comments

@shibd
Copy link
Member

shibd commented Aug 19, 2022

Search before asking

  • I searched in the issues and found nothing similar.

Motivation

PIP 132 Include message header size when check maxMessageSize for non-batch message on the client side

Current c++ the implementation also has this problem.

    const bool compressed = !canAddToBatch(msg);
    const auto payload =
        compressed ? applyCompression(uncompressedPayload, conf_.getCompressionType()) : uncompressedPayload;
    const auto compressedSize = static_cast<uint32_t>(payload.readableBytes());
    const auto maxMessageSize = static_cast<uint32_t>(ClientConnection::getMaxMessageSize());

    if (compressed && compressedSize > ClientConnection::getMaxMessageSize() && !chunkingEnabled_) {
        LOG_WARN(getName() << " - compressed Message payload size " << payload.readableBytes()
                           << " cannot exceed " << ClientConnection::getMaxMessageSize()
                           << " bytes unless chunking is enabled");
        handleFailedResult(ResultMessageTooBig);
        return;
    }

Solution

Refer java client PR: #14007

Alternatives

No response

Anything else?

This is a PR to match the Java implementation. It is best to first prove whether the problem exists in the C++ client, and then solve it.

Are you willing to submit a PR?

  • I'm willing to submit a PR!
@coderzc
Copy link
Member

coderzc commented Aug 23, 2022

Please assign it to me.

RobertIndie pushed a commit that referenced this issue Sep 22, 2022
…axMessageSize (#17289)

### Motivation

See: #17188

### Modifications

Support include message header size when check maxMessageSize for cpp client
@github-actions
Copy link

The issue had no activity for 30 days, mark with Stale label.

@github-actions github-actions bot added the Stale label Sep 23, 2022
@shibd shibd closed this as completed Oct 27, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

3 participants