Skip to content

[Issue 4803][client] return null if the message value/data is not set by producer #6379

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.

Sign up for GitHub

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

Jump to bottom
Merged
codelipenghui merged 10 commits into from
May 19, 2020
Merged

[Issue 4803][client] return null if the message value/data is not set by producer #6379

codelipenghui merged 10 commits into from
May 19, 2020

Conversation

@nlu90
Copy link
Member

@nlu90 nlu90 commented Feb 20, 2020

Fixes #4803

Motivation

Allow the typed consumer receive messages with null value if the producer sends message without payload.

Modifications

  • add a flag in MessageMetadata to indicate if the payload is set when the message is created
  • check and return null if the flag is not set when reading data from a message

Verifying this change

  • Make sure that the change passes the CI checks.

This change added tests and can be verified as follows:

  • Added unit test testTypedSchemaGetNullValue in MessageImplTest.java

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): no
  • The public API: no
  • The schema: no
  • The default values of configurations: no
  • The wire protocol: no
  • The rest endpoints: no
  • The admin cli options: no
  • Anything that affects deployment: no

Documentation

  • Does this pull request introduce a new feature? no
  • If yes, how is the feature documented? (not applicable / docs / JavaDocs / not documented)
  • If a feature is not applicable for documentation, explain why?
  • If a feature is not documented yet in this PR, please create a followup issue for adding the documentation

@nlu90 nlu90 requested a review from sijie February 20, 2020 05:38
@nlu90
Copy link
Member Author

nlu90 commented Feb 20, 2020

Thanks @congbobo184 for his PR #4848 to provide context and unit test reference.

@sijie sijie added this to the 2.6.0 milestone Feb 22, 2020
@sijie
Copy link
Member

sijie commented Apr 5, 2020

/pulsarbot run-failure-checks

@gaoran10 gaoran10 force-pushed the neng/consume-message-with-null-value branch from 37710c8 to 543b55f Compare April 24, 2020 18:32
gaoran10 added 2 commits April 25, 2020 02:36
@gaoran10
add javadoc
7b6019f
@gaoran10
Add flag valueIsSet indicate users whether invoke the method `Typed…
449e45c 
…MessageBuilder<T> value(T value)` of the Class `TypedMessageBuilderImpl`.
@gaoran10
Copy link
Contributor

gaoran10 commented Apr 24, 2020 •
edited
Loading

@nlu90 Hi, I'm working on this feature based on your PR. Thanks.

Add the flag nullValue in the MessageMetadata and SingleMessageMetadata default value is false. If the message value is null set the flag to true. When users invoke the method message.getValue() or message.getData(), then check the nullValue flag, if the flag is true return null.

Support users to peek the null value message by admin API.

Users could send a null value message as below.

producer.newMessage().value(null).send();

@nlu90 @sijie @jiazhai @codelipenghui Please have a review, thanks.

@codelipenghui
Copy link
Contributor

codelipenghui commented Apr 25, 2020

/pulsarbot run-failure-checks

@gaoran10
Copy link
Contributor

gaoran10 commented Apr 27, 2020

/pulsarbot run-failure-checks

@gaoran10
Copy link
Contributor

gaoran10 commented May 6, 2020

/pulsarbot run-failure-checks

2 similar comments
@congbobo184
Copy link
Contributor

congbobo184 commented May 7, 2020

/pulsarbot run-failure-checks

@gaoran10
Copy link
Contributor

gaoran10 commented May 9, 2020

/pulsarbot run-failure-checks

@codelipenghui
Copy link
Contributor

codelipenghui commented May 19, 2020

@nlu90 @gaoran10 Thanks for the fix.

@codelipenghui codelipenghui merged commit d55bc00 into apache:master May 19, 2020
This was referenced May 19, 2020
Closed
Closed
Huanli-Meng pushed a commit to Huanli-Meng/pulsar that referenced this pull request May 27, 2020
@nlu90 @Huanli-Meng
[Issue 4803][client] return null if the message value/data is not set…
235bd9f 
… by producer (apache#6379)

Fixes apache#4803 

### Motivation
Allow the typed consumer receive messages with `null` value if the producer sends message without payload.

### Modifications
- add a flag in `MessageMetadata` to indicate if the payload is set when the message is created
- check and return `null` if the flag is not set when reading data from a message
huangdx0726 pushed a commit to huangdx0726/pulsar that referenced this pull request Aug 24, 2020
@nlu90 @huangdx0726
[Issue 4803][client] return null if the message value/data is not set…
4f81ea3 
… by producer (apache#6379)

Fixes apache#4803 

### Motivation
Allow the typed consumer receive messages with `null` value if the producer sends message without payload.

### Modifications
- add a flag in `MessageMetadata` to indicate if the payload is set when the message is created
- check and return `null` if the flag is not set when reading data from a message
@BewareMyPower BewareMyPower mentioned this pull request Jan 4, 2021
Merged
1 task
sijie pushed a commit that referenced this pull request Jan 4, 2021
@BewareMyPower
Fix NPE when MultiTopicsConsumerImpl receives null value messages (#9113
dd3b9d8 
)

### Motivation

[#6379](#6379) introduced the feature to handle null value messages, but it only checks the null value in `ConsumerImpl` when `INCOMING_MESSAGES_SIZE_UPDATER` is updated. Therefore, if a partitioned topic with at least 2 partitions was consumed with a null value message, the NPE would be thrown.

### Modifications

- Check the null value message in `MultiTopicsConsumerImpl` as well as `ConsumerImpl`. To reduce repeated code, two protected methods are added to `ConsumerBase` and `INCOMING_MESSAGES_SIZE_UPDATER` becomes private now, the derived consumer classes just use these two methods to update or reset `INCOMING_MESSAGES_SIZE_UPDATER`.
- Add tests for partitioned topics in `NullValueTest`. Since the existed tests rely on the message send order, here we only send messages to a single partition only.
codelipenghui pushed a commit that referenced this pull request Jan 6, 2021
@BewareMyPower @codelipenghui
Fix NPE when MultiTopicsConsumerImpl receives null value messages (#9113
68793d5 
)

[#6379](#6379) introduced the feature to handle null value messages, but it only checks the null value in `ConsumerImpl` when `INCOMING_MESSAGES_SIZE_UPDATER` is updated. Therefore, if a partitioned topic with at least 2 partitions was consumed with a null value message, the NPE would be thrown.

- Check the null value message in `MultiTopicsConsumerImpl` as well as `ConsumerImpl`. To reduce repeated code, two protected methods are added to `ConsumerBase` and `INCOMING_MESSAGES_SIZE_UPDATER` becomes private now, the derived consumer classes just use these two methods to update or reset `INCOMING_MESSAGES_SIZE_UPDATER`.
- Add tests for partitioned topics in `NullValueTest`. Since the existed tests rely on the message send order, here we only send messages to a single partition only.

(cherry picked from commit dd3b9d8)
codelipenghui pushed a commit that referenced this pull request Jan 7, 2021
@BewareMyPower @codelipenghui
Fix NPE when MultiTopicsConsumerImpl receives null value messages (#9113
9b5a4b2 
)

### Motivation

[#6379](#6379) introduced the feature to handle null value messages, but it only checks the null value in `ConsumerImpl` when `INCOMING_MESSAGES_SIZE_UPDATER` is updated. Therefore, if a partitioned topic with at least 2 partitions was consumed with a null value message, the NPE would be thrown.

### Modifications

- Check the null value message in `MultiTopicsConsumerImpl` as well as `ConsumerImpl`. To reduce repeated code, two protected methods are added to `ConsumerBase` and `INCOMING_MESSAGES_SIZE_UPDATER` becomes private now, the derived consumer classes just use these two methods to update or reset `INCOMING_MESSAGES_SIZE_UPDATER`.
- Add tests for partitioned topics in `NullValueTest`. Since the existed tests rely on the message send order, here we only send messages to a single partition only.

(cherry picked from commit dd3b9d8)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@sijie sijie sijie approved these changes

@codelipenghui codelipenghui codelipenghui approved these changes

@jiazhai jiazhai Awaiting requested review from jiazhai

Assignees

@gaoran10 gaoran10

@nlu90 nlu90

Labels

area/client

Projects

None yet

Milestone

2.6.0

Development

Successfully merging this pull request may close these issues.

Pulsar schema doesn't support null value

5 participants

@nlu90 @sijie @gaoran10 @codelipenghui @congbobo184