Skip to content

Conversation

@coderzc
Copy link
Member

@coderzc coderzc commented Aug 26, 2022

Master Issue: #17188

Motivation

See: #17188

Modifications

Support include message header size when check maxMessageSize for cpp client

Verifying this change

  • Make sure that the change passes the CI checks.

This change is already covered by existing tests, such as testMaxMessageSizetestNoBatchMaxMessageSizetestChunkingMaxMessageSize.

Does this pull request potentially affect one of the following parts:

If yes was chosen, please highlight the changes

  • Dependencies (does it add or upgrade a dependency): (yes / no)
  • The public API: (yes / no)
  • The schema: (yes / no / don't know)
  • The default values of configurations: (yes / no)
  • The wire protocol: (yes / no)
  • The rest endpoints: (yes / no)
  • The admin cli options: (yes / no)
  • Anything that affects deployment: (yes / no / don't know)

Documentation

Check the box below or label this PR directly.

Need to update docs?

  • doc-required
    (Your PR needs to update docs and you will update later)

  • doc-not-needed
    (Please explain why)

  • doc
    (Your PR contains doc changes)

  • doc-complete
    (Docs have been already added)

@github-actions github-actions bot added the doc-not-needed Your PR changes do not impact docs label Aug 26, 2022
@shibd
Copy link
Member

shibd commented Aug 29, 2022

Odd, why is the java client compilation failed?
https://github.com/apache/pulsar/runs/8033665923?check_suite_focus=true

Error:  Failed to execute goal org.apache.maven.plugins:maven-compiler-plugin:3.10.1:compile (default-compile) on project pulsar-client-original: Compilation failure
Error:  /Users/runner/work/pulsar/pulsar/pulsar-client/src/main/java/org/apache/pulsar/client/impl/BatchMessageContainerImpl.java:[222,39] no suitable method found for sendMessage(long,long,int,org.apache.pulsar.common.api.proto.MessageMetadata,io.netty.buffer.ByteBuf)
Error:      method org.apache.pulsar.client.impl.ProducerImpl.sendMessage(long,long,int,org.apache.pulsar.client.api.MessageId,org.apache.pulsar.common.api.proto.MessageMetadata,io.netty.buffer.ByteBuf) is not applicable
Error:        (actual and formal argument lists differ in length)
Error:      method org.apache.pulsar.client.impl.ProducerImpl.sendMessage(long,long,long,int,org.apache.pulsar.common.api.proto.MessageMetadata,io.netty.buffer.ByteBuf) is not applicable

@coderzc
Copy link
Member Author

coderzc commented Aug 29, 2022

Odd, why is the java client compilation failed? https://github.com/apache/pulsar/runs/8033665923?check_suite_focus=true

Error:  Failed to execute goal org.apache.maven.plugins:maven-compiler-plugin:3.10.1:compile (default-compile) on project pulsar-client-original: Compilation failure
Error:  /Users/runner/work/pulsar/pulsar/pulsar-client/src/main/java/org/apache/pulsar/client/impl/BatchMessageContainerImpl.java:[222,39] no suitable method found for sendMessage(long,long,int,org.apache.pulsar.common.api.proto.MessageMetadata,io.netty.buffer.ByteBuf)
Error:      method org.apache.pulsar.client.impl.ProducerImpl.sendMessage(long,long,int,org.apache.pulsar.client.api.MessageId,org.apache.pulsar.common.api.proto.MessageMetadata,io.netty.buffer.ByteBuf) is not applicable
Error:        (actual and formal argument lists differ in length)
Error:      method org.apache.pulsar.client.impl.ProducerImpl.sendMessage(long,long,long,int,org.apache.pulsar.common.api.proto.MessageMetadata,io.netty.buffer.ByteBuf) is not applicable

See: #17300

@coderzc coderzc requested a review from BewareMyPower August 30, 2022 04:38
@coderzc coderzc force-pushed the check_message_header_cpp branch from c4d2953 to 4ea30f5 Compare September 8, 2022 08:30
@coderzc
Copy link
Member Author

coderzc commented Sep 20, 2022

@RobertIndie PTAL.

@BewareMyPower
Copy link
Contributor

@RobertIndie @merlimat @Demogorgon314 Could any of you leave a 2nd review?

@RobertIndie RobertIndie merged commit f28f985 into apache:master Sep 22, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

doc-not-needed Your PR changes do not impact docs

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants