Skip to content

MINOR: fix LogValidatorTest#checkNonCompressed#15904

Merged
chia7712 merged 5 commits intoapache:trunkfrom
chia7712:MINOR-15904
May 11, 2024
Merged

MINOR: fix LogValidatorTest#checkNonCompressed#15904
chia7712 merged 5 commits intoapache:trunkfrom
chia7712:MINOR-15904

Conversation

@chia7712
Copy link
Copy Markdown
Member

@chia7712 chia7712 commented May 9, 2024

from: #15621 (comment)

This is a follow-up of #15621

Committer Checklist (excluded from commit message)

  • Verify design and implementation
  • Verify test coverage and CI build status
  • Verify documentation (including upgrade notes)

Copy link
Copy Markdown
Contributor

@junrao junrao left a comment

Choose a reason for hiding this comment

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

@chia7712 : Thanks for the PR. Left a couple of comments.

Comment thread core/src/main/scala/kafka/log/UnifiedLog.scala Outdated
Comment thread core/src/test/scala/unit/kafka/log/LogValidatorTest.scala Outdated
@chia7712
Copy link
Copy Markdown
Member Author

@junrao thanks for reviewing this rough patch :(

Copy link
Copy Markdown
Contributor

@junrao junrao left a comment

Choose a reason for hiding this comment

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

@chia7712 : Thanks for the updated PR. LGTM

Have the 30 test failures been triaged?

@chia7712
Copy link
Copy Markdown
Member Author

Have the 30 test failures been triaged?

test jira
testSyncTopicConfigs https://issues.apache.org/jira/browse/KAFKA-15945
testReplicateSourceDefault https://issues.apache.org/jira/browse/KAFKA-15292
testProduceConsumeWithWildcardAcls https://issues.apache.org/jira/browse/KAFKA-16697
testFenceMultipleBrokers https://issues.apache.org/jira/browse/KAFKA-16634
testSeparateOffsetsTopic https://issues.apache.org/jira/browse/KAFKA-14089
testNoDescribeProduceOrConsumeWithoutTopicDescribeAcl https://issues.apache.org/jira/browse/KAFKA-8250
testDescribeQuorumReplicationSuccessful https://issues.apache.org/jira/browse/KAFKA-15104
testDescribeQuorumStatusSuccessful https://issues.apache.org/jira/browse/KAFKA-16174
testTaskRequestWithOldStartMsGetsUpdated https://issues.apache.org/jira/browse/KAFKA-16136
testConsumptionWithBrokerFailures https://issues.apache.org/jira/browse/KAFKA-15146
testCoordinatorFailover https://issues.apache.org/jira/browse/KAFKA-16024
testBrokerHeartbeatDuringMigration https://issues.apache.org/jira/browse/KAFKA-15963
testAbortTransactionTimeout https://issues.apache.org/jira/browse/KAFKA-15772
testMultiConsumerStickyAssignor https://issues.apache.org/jira/browse/KAFKA-15934
testDynamicIpConnectionRateQuota https://issues.apache.org/jira/browse/KAFKA-16698
testCreateClusterAndPerformReassignment https://issues.apache.org/jira/browse/KAFKA-15103
shouldBootstrapTwoBrokersWithFollowerThrottle https://issues.apache.org/jira/browse/KAFKA-4184

Besides, the thread leaks should be fixed by #15886. Hence, I will rebase code to trigger QA again.

@chia7712
Copy link
Copy Markdown
Member Author

@chia7712 chia7712 merged commit 4dff60d into apache:trunk May 11, 2024

// Both V2 and V1 has single branch in the record when compression is enable, and hence their shallow OffsetOfMaxTimestamp
// is the last offset of the single branch
assertEquals(1, records.batches().asScala.size)
Copy link
Copy Markdown
Contributor

@vincent81jiang vincent81jiang May 13, 2024

Choose a reason for hiding this comment

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

If I understand this test case correctly, there might still be a minor issue with "checkRecompression":

  1. at line 455, records should be created with "CompressionType.NONE".
  2. at line 502, the check should be assertEquals(1, validatedRecords.batches().asScala.size)

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.

@vincent81jiang : Thanks for the comment. The line at 455 seems to be wrong. What do you think @chia7712 ?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

@vincent81jiang you are totally right. I have filed PR (#15948) to fix that

gongxuanzhang pushed a commit to gongxuanzhang/kafka that referenced this pull request Jun 12, 2024
@chia7712 chia7712 deleted the MINOR-15904 branch September 13, 2024 07:35
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.

3 participants