You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@pulsar.apache.org by "Bhargavkonidena (via GitHub)" <gi...@apache.org> on 2026/03/30 13:18:52 UTC
[PR] [feat][client] Support null value messages [pulsar-client-cpp]
Bhargavkonidena opened a new pull request, #563:
URL: https://github.com/apache/pulsar-client-cpp/pull/563
<!--
### Contribution Checklist
- PR title format should be *[type][component] summary*. For details, see *[Guideline - Pulsar PR Naming Convention](https://docs.google.com/document/d/1d8Pw6ZbWk-_pCKdOmdvx9rnhPiyuxwq60_TrD68d7BA/edit#heading=h.trs9rsex3xom)*.
- Fill out the template below to describe the changes contributed by the pull request. That will give reviewers the context they need to do the review.
- Each pull request should address only one issue, not mix up code from multiple issues.
- Each commit in the pull request has a meaningful commit message
- Once all items of the checklist are addressed, remove the above text and this checklist, leaving only the filled out template below.
-->
<!-- Either this PR fixes an issue, -->
Fixes #489
<!-- or this PR is one task of an issue -->
### Motivation
Currently the producer cannot send a null value message, which is used as a tombstone for a specific key on a compacted topic.
In addition, the consumer cannot differentiate empty values from null values, because all these values are presented as empty std::strings.
### Modifications
1. Added MessageBuilder::setNullValue() method that sets the null_value field in MessageMetadata
2. Added Message::hasNullValue() method to check if a message has null value
3. Updated batch message serialization (Commands.cc) to preserve null_value in SingleMessageMetadata
4. Updated batch message deserialization (Message.cc) to copy null_value from SingleMessageMetadata
5. Added C API functions: pulsar_message_set_null_value() and pulsar_message_has_null_value()
6. Added unit tests and integration tests
### Verifying this change
- [ ] Make sure that the change passes the CI checks.
*(Please pick either of the following options)*
This change is a trivial rework / code cleanup without any test coverage.
*(or)*
This change is already covered by existing tests, such as *(please describe tests)*.
*(or)*
This change added tests and can be verified as follows:
*(example:)*
- *Added integration tests for end-to-end deployment with large payloads (10MB)*
- *Extended integration test for recovery after broker failure*
### Documentation
<!-- DO NOT REMOVE THIS SECTION. CHECK THE PROPER BOX ONLY. -->
- [ ] `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)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [feat][client] Support null value messages [pulsar-client-cpp]
Posted by "BewareMyPower (via GitHub)" <gi...@apache.org>.
BewareMyPower commented on PR #563:
URL: https://github.com/apache/pulsar-client-cpp/pull/563#issuecomment-4175992191
Please merge this branch to the latest main branch to have the fix included
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [feat][client] Support null value messages [pulsar-client-cpp]
Posted by "Bhargavkonidena (via GitHub)" <gi...@apache.org>.
Bhargavkonidena commented on PR #563:
URL: https://github.com/apache/pulsar-client-cpp/pull/563#issuecomment-4177624913
> Please merge this branch to the latest main branch to have the fix included
Done
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [feat][client] Support null value messages [pulsar-client-cpp]
Posted by "BewareMyPower (via GitHub)" <gi...@apache.org>.
BewareMyPower commented on PR #563:
URL: https://github.com/apache/pulsar-client-cpp/pull/563#issuecomment-4175430358
The fix is here: https://github.com/apache/pulsar-client-cpp/pull/565
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [feat][client] Support null value messages [pulsar-client-cpp]
Posted by "Bhargavkonidena (via GitHub)" <gi...@apache.org>.
Bhargavkonidena commented on PR #563:
URL: https://github.com/apache/pulsar-client-cpp/pull/563#issuecomment-4175141157
> There are some tests that are very flaky, I will take a look first
Sure @BewareMyPower, please let me know once its done from your end.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [feat][client] Support null value messages [pulsar-client-cpp]
Posted by "BewareMyPower (via GitHub)" <gi...@apache.org>.
BewareMyPower commented on PR #563:
URL: https://github.com/apache/pulsar-client-cpp/pull/563#issuecomment-4174982338
There are some tests that are very flaky, I will take a look first
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [feat][client] Support null value messages [pulsar-client-cpp]
Posted by "Bhargavkonidena (via GitHub)" <gi...@apache.org>.
Bhargavkonidena commented on PR #563:
URL: https://github.com/apache/pulsar-client-cpp/pull/563#issuecomment-4183228457
Looks like, the failure is again due to transient networking issue.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [feat][client] Support null value messages [pulsar-client-cpp]
Posted by "Copilot (via GitHub)" <gi...@apache.org>.
Copilot commented on code in PR #563:
URL: https://github.com/apache/pulsar-client-cpp/pull/563#discussion_r3011110803
##########
tests/ReaderTest.cc:
##########
@@ -1045,5 +1045,130 @@ TEST(ReaderTest, testReaderWithZeroMessageListenerThreads) {
client.close();
}
+TEST(ReaderTest, testReadCompactedWithNullValue) {
+ Client client(serviceUrl);
+
+ const std::string topicName =
+ "persistent://public/default/testReadCompactedWithNullValue-" + std::to_string(time(nullptr));
+
+ Producer producer;
+ ASSERT_EQ(ResultOk, client.createProducer(topicName, producer));
+
+ // Send messages with keys
+ ASSERT_EQ(ResultOk,
+ producer.send(MessageBuilder().setPartitionKey("key1").setContent("value1").build()));
+ ASSERT_EQ(ResultOk,
+ producer.send(MessageBuilder().setPartitionKey("key2").setContent("value2").build()));
+ ASSERT_EQ(ResultOk,
+ producer.send(MessageBuilder().setPartitionKey("key3").setContent("value3").build()));
+
+ // Send a tombstone (null value) for key2
+ auto tombstone = MessageBuilder().setPartitionKey("key2").setNullValue().build();
+ ASSERT_TRUE(tombstone.hasNullValue());
+ ASSERT_EQ(tombstone.getLength(), 0);
+ ASSERT_EQ(ResultOk, producer.send(tombstone));
+
+ // Update key1 with a new value
+ ASSERT_EQ(ResultOk,
+ producer.send(MessageBuilder().setPartitionKey("key1").setContent("value1-updated").build()));
+
+ // Trigger compaction via admin API
+ {
+ std::string compactUrl =
+ adminUrl + "admin/v2/persistent/public/default/testReadCompactedWithNullValue-" +
+ std::to_string(time(nullptr)) + "/compaction";
+ // Note: Compaction is async, we just trigger it
+ makePutRequest(compactUrl, "");
+ }
+
+ // Create a reader with readCompacted enabled
+ ReaderConfiguration readerConf;
+ readerConf.setReadCompacted(true);
+ Reader reader;
+ ASSERT_EQ(ResultOk, client.createReader(topicName, MessageId::earliest(), readerConf, reader));
+
+ // Read all messages and verify we can detect null values
+ std::map<std::string, std::string> keyValues;
+ std::set<std::string> nullValueKeys;
+
+ for (int i = 0; i < 10; i++) {
+ bool hasMessageAvailable = false;
+ ASSERT_EQ(ResultOk, reader.hasMessageAvailable(hasMessageAvailable));
+ if (!hasMessageAvailable) {
+ break;
+ }
+
+ Message msg;
+ Result res = reader.readNext(msg, 3000);
+ if (res != ResultOk) {
+ break;
+ }
Review Comment:
This read loop can silently succeed without validating behavior: it breaks when `hasMessageAvailable` is false or `readNext` fails, and the final assertion can still pass even if no messages were read at all. Prefer asserting `readNext` succeeds while `hasMessageAvailable==true` (or reading a known number of messages), and then assert on concrete outcomes (e.g., key1 == "value1-updated" and key2 is tombstone/absent after compaction).
```suggestion
ASSERT_EQ(ResultOk, reader.readNext(msg, 3000));
```
##########
lib/MessageBuilder.cc:
##########
@@ -157,6 +157,12 @@ MessageBuilder& MessageBuilder::disableReplication(bool flag) {
return *this;
}
+MessageBuilder& MessageBuilder::setNullValue() {
+ checkMetadata();
Review Comment:
`MessageBuilder::setNullValue()` only flips the metadata flag; it doesn't clear any previously-set payload, and the various `setContent(...)` overloads don't clear `null_value`. This allows building messages with non-empty payload but `null_value=true` (or vice versa), which will be interpreted incorrectly by consumers/table views. Make the two states mutually exclusive by clearing the payload/key-value when setting null value, and clearing `null_value` whenever content is set.
```suggestion
checkMetadata();
// Ensure null-value messages do not carry any payload or key-related metadata.
// Clear the payload buffer.
impl_->payload = SharedBuffer();
// Clear key-related metadata so that the null value state is self-consistent.
impl_->metadata.clear_partition_key();
impl_->metadata.clear_ordering_key();
```
##########
tests/ReaderTest.cc:
##########
@@ -1045,5 +1045,130 @@ TEST(ReaderTest, testReaderWithZeroMessageListenerThreads) {
client.close();
}
+TEST(ReaderTest, testReadCompactedWithNullValue) {
+ Client client(serviceUrl);
+
+ const std::string topicName =
+ "persistent://public/default/testReadCompactedWithNullValue-" + std::to_string(time(nullptr));
+
+ Producer producer;
+ ASSERT_EQ(ResultOk, client.createProducer(topicName, producer));
+
+ // Send messages with keys
+ ASSERT_EQ(ResultOk,
+ producer.send(MessageBuilder().setPartitionKey("key1").setContent("value1").build()));
+ ASSERT_EQ(ResultOk,
+ producer.send(MessageBuilder().setPartitionKey("key2").setContent("value2").build()));
+ ASSERT_EQ(ResultOk,
+ producer.send(MessageBuilder().setPartitionKey("key3").setContent("value3").build()));
+
+ // Send a tombstone (null value) for key2
+ auto tombstone = MessageBuilder().setPartitionKey("key2").setNullValue().build();
+ ASSERT_TRUE(tombstone.hasNullValue());
+ ASSERT_EQ(tombstone.getLength(), 0);
+ ASSERT_EQ(ResultOk, producer.send(tombstone));
+
+ // Update key1 with a new value
+ ASSERT_EQ(ResultOk,
+ producer.send(MessageBuilder().setPartitionKey("key1").setContent("value1-updated").build()));
+
+ // Trigger compaction via admin API
+ {
+ std::string compactUrl =
+ adminUrl + "admin/v2/persistent/public/default/testReadCompactedWithNullValue-" +
+ std::to_string(time(nullptr)) + "/compaction";
+ // Note: Compaction is async, we just trigger it
+ makePutRequest(compactUrl, "");
Review Comment:
The compaction admin URL is built using a fresh `time(nullptr)` value, which can differ from the one used to construct `topicName`. This can trigger compaction on a different (nonexistent) topic than the one you produced to. Build `compactUrl` directly from `topicName` (or extract the topic suffix once) and assert the returned HTTP status from `makePutRequest` (e.g., 204/409) so the test fails if compaction isn't triggered.
```suggestion
// Build compaction URL directly from topicName to avoid mismatches
std::string topicPath = topicName;
std::size_t schemePos = topicPath.find("://");
if (schemePos != std::string::npos) {
topicPath.erase(schemePos, 3); // remove "://", e.g., "persistent://public/..." -> "persistent/public/..."
}
std::string compactUrl = adminUrl + "admin/v2/" + topicPath + "/compaction";
// Note: Compaction is async, we just trigger it, but assert that the request is accepted
int res = makePutRequest(compactUrl, "");
ASSERT_FALSE(res != 204 && res != 409);
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [feat][client] Support null value messages [pulsar-client-cpp]
Posted by "Bhargavkonidena (via GitHub)" <gi...@apache.org>.
Bhargavkonidena commented on PR #563:
URL: https://github.com/apache/pulsar-client-cpp/pull/563#issuecomment-4181776747
@BewareMyPower
Looks like, the failure is a transient infrastructure issue - GitHub had a temporary network problem downloading the `abseil` library dependency. Could you please re-run the workflow.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [feat][client] Support null value messages [pulsar-client-cpp]
Posted by "BewareMyPower (via GitHub)" <gi...@apache.org>.
BewareMyPower merged PR #563:
URL: https://github.com/apache/pulsar-client-cpp/pull/563
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [feat][client] Support null value messages [pulsar-client-cpp]
Posted by "Bhargavkonidena (via GitHub)" <gi...@apache.org>.
Bhargavkonidena commented on PR #563:
URL: https://github.com/apache/pulsar-client-cpp/pull/563#issuecomment-4175636565
> The fix is here: #565
great @BewareMyPower Could you please provide approval to run CI checks for this PR once your fix is merged.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [feat][client] Support null value messages [pulsar-client-cpp]
Posted by "BewareMyPower (via GitHub)" <gi...@apache.org>.
BewareMyPower commented on PR #563:
URL: https://github.com/apache/pulsar-client-cpp/pull/563#issuecomment-4175109246
https://github.com/apache/pulsar-client-cpp/issues/564 I'm working on the broken CI currently
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [feat][client] Support null value messages [pulsar-client-cpp]
Posted by "Bhargavkonidena (via GitHub)" <gi...@apache.org>.
Bhargavkonidena commented on code in PR #563:
URL: https://github.com/apache/pulsar-client-cpp/pull/563#discussion_r3019801887
##########
lib/MessageBuilder.cc:
##########
@@ -157,6 +157,12 @@ MessageBuilder& MessageBuilder::disableReplication(bool flag) {
return *this;
}
+MessageBuilder& MessageBuilder::setNullValue() {
+ checkMetadata();
Review Comment:
I've implemented clearing the payload buffer (impl_->payload = SharedBuffer()) and made setContent() clear the null_value flag for mutual exclusivity.
However, I intentionally did not implement clearing partition_key and ordering_key as suggested. Here's why:
Tombstone messages in Pulsar require a partition key to work with topic compaction - the key identifies which message is being "deleted".
```
auto msg = MessageBuilder().setPartitionKey("key2").setNullValue().build();
ASSERT_TRUE(msg.hasNullValue());
ASSERT_EQ(msg.getPartitionKey(), "key2"); // Key must be preserved!
```
And the integration test relies on this:
```
auto tombstone = MessageBuilder().setPartitionKey("key2").setNullValue().build();
producer.send(tombstone); // Tombstone for key2 in compacted topic
```
Clearing the keys would break the tombstone/compaction functionality. This matches the behavior of the Java client where tombstones retain their keys.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org