Skip to content

NIFI-16517 Do not mark a non-null empty Kafka value as a tombstone in FLOW_FILE processing strategy- #11860 - #11899

Draft
rzepinskip wants to merge 1 commit into
apache:mainfrom
rzepinskip:NIFI-16517
Draft

rzepinskip wants to merge 1 commit into
apache:mainfrom
rzepinskip:NIFI-16517

Conversation

@rzepinskip

Copy link
Copy Markdown
Contributor

Summary

NIFI-16517

Null values were rewritten to an empty byte array, so FLOW_FILE processing could not tell a tombstone from an empty payload. Keep that distinction on ByteRecord and set kafka.tombstone only for a null value.

Tracking

Please complete the following tracking steps prior to pull request creation.

Issue Tracking

Pull Request Tracking

  • Pull Request title starts with Apache NiFi Jira issue number, such as NIFI-00000
  • Pull Request commit message starts with Apache NiFi Jira issue number, as such NIFI-00000
  • Pull request contains commits signed with a registered key indicating Verified status

Pull Request Formatting

  • Pull Request based on current revision of the main branch
  • Pull Request refers to a feature branch with one commit containing changes

Verification

Please indicate the verification steps performed prior to pull request creation.

Build

  • Build completed using ./mvnw clean install -P contrib-check
    • JDK 21
    • JDK 25

Licensing

  • New dependencies are compatible with the Apache License 2.0 according to the License Policy
  • New dependencies are documented in applicable LICENSE and NOTICE files

Documentation

  • Documentation formatting appears as expected in rendered files

@rzepinskip
rzepinskip force-pushed the NIFI-16517 branch 2 times, most recently from d5b7a62 to 1d7bf47 Compare November 10, 2026 10:07
…LOW_FILE

A null Kafka value is a tombstone. A non-null empty value is an ordinary empty payload and no longer sets kafka.tombstone in FLOW_FILE processing strategy. DEMARCATOR keeps inferring a tombstone from an empty bundle.

Co-authored-by: Cursor <cursoragent@cursor.com>
Comment on lines +68 to +72
final byte[] data = value.getData();
// Demarcator emits one FlowFile per bundle, so an empty bundle is still reported as a tombstone.
final boolean tombstone = data.length == 0;
return new ByteRecord(topicPartition.getTopic(), topicPartition.getPartition(),
value.getFirstOffset(), key.getTimestamp(), key.getHeaders(), key.getMessageKey(), value.getData(), value.getCount(), false);
value.getFirstOffset(), key.getTimestamp(), key.getHeaders(), key.getMessageKey(), data, value.getCount(), tombstone);

@rzepinskip rzepinskip Nov 10, 2026 •

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Because FLOW_FILE and DEMARCATOR use FlowFileStreamKafkaMessageConverter.java‎ these changes are required to preserve existing behavior for DEMARCATOR:

Records in one bundle FlowFile content
hello, tombstone, world hello||world
hello, tombstone hello|
tombstone, hello, world hello|world
tombstone only (or several tombstones only) empty + tombstone attribute
empty empty + tombstone attribute

Added tests as well.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Alternative is to skip this code change and have change in behavior (bundles with empty / null tombstone only would result in empty flow file without tombstone attribute set).

Proper support for DEMARCATOR is left for separate ticket: https://issues.apache.org/jira/browse/NIFI-16521

runner.run(1, false, true);

produceBytes(topic, FIRST_PARTITION, "key-record", EMPTY_BYTES);
while (runner.getFlowFilesForRelationship(ConsumeKafka.SUCCESS).isEmpty()) {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let's use runUntil method to stop execution if the condition is never met.

Comment on lines +50 to +57
final ProcessSession session = mock(ProcessSession.class, withSettings().defaultAnswer(Answers.RETURNS_DEEP_STUBS));
final FlowFile tombstoneFlowFile = new MockFlowFile(1);
final FlowFile emptyFlowFile = new MockFlowFile(2);
final FlowFile valueFlowFile = new MockFlowFile(3);
when(session.create()).thenReturn(tombstoneFlowFile, emptyFlowFile, valueFlowFile);
when(session.putAttribute(any(FlowFile.class), any(), any())).thenAnswer(invocation -> invocation.getArgument(0));
when(session.putAllAttributes(any(FlowFile.class), any())).thenAnswer(invocation -> invocation.getArgument(0));
when(session.write(any(FlowFile.class), any(OutputStreamCallback.class))).thenAnswer(invocation -> invocation.getArgument(0));

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I suggest using MockProcessSession instead of handcrafted mocks.

Comment on lines +69 to +70
// Demarcator emits one FlowFile per bundle, so an empty bundle is still reported as a tombstone.
final boolean tombstone = data.length == 0;

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nit: let's mention that this is done for compatibility. It's hard to tell why we check for empty content from the code alone.

import static org.junit.jupiter.api.Assertions.assertEquals;

/**
* Demarcator output when a batch contains a null tombstone, a non-null empty value, or both together with a real value.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nit: the verb is missing, so I had some troubles understanding the sentence.

Suggested change
* Demarcator output when a batch contains a null tombstone, a non-null empty value, or both together with a real value.
* Verifies DEMARCATOR output when a batch contains a null tombstone, a non-null empty value, or both together with a real value.

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.

2 participants