Repository navigation
NIFI-16517 Do not mark a non-null empty Kafka value as a tombstone in FLOW_FILE processing strategy- #11860 - #11899
rzepinskip wants to merge 1 commit into
Conversation
d5b7a62 to
1d7bf47
Compare
…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>
1d7bf47 to
0014d0e
Compare
| 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); |
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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()) { |
There was a problem hiding this comment.
Let's use runUntil method to stop execution if the condition is never met.
| 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)); |
There was a problem hiding this comment.
I suggest using MockProcessSession instead of handcrafted mocks.
| // Demarcator emits one FlowFile per bundle, so an empty bundle is still reported as a tombstone. | ||
| final boolean tombstone = data.length == 0; |
There was a problem hiding this comment.
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. |
There was a problem hiding this comment.
Nit: the verb is missing, so I had some troubles understanding the sentence.
| * 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. |
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
NIFI-00000NIFI-00000VerifiedstatusPull Request Formatting
mainbranchVerification
Please indicate the verification steps performed prior to pull request creation.
Build
./mvnw clean install -P contrib-checkLicensing
LICENSEandNOTICEfilesDocumentation