Skip to content

KAFKA-7484: fix suppression integration tests - #5748

Merged
mjsax merged 1 commit into
apache:trunkfrom
vvcephei:fix-suppress-integration-tests
Oct 5, 2018
Merged

KAFKA-7484: fix suppression integration tests#5748
mjsax merged 1 commit into
apache:trunkfrom
vvcephei:fix-suppress-integration-tests

Conversation

@vvcephei

@vvcephei vvcephei commented Oct 5, 2018

Copy link
Copy Markdown
Contributor

The test data is based at time 0 (aka 1970), but the broker was started at time now.
This caused the log cleaner to delete the test data if the tests took slightly too long, despite its generous default configuration.

Committer Checklist (excluded from commit message)

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

@vvcephei

vvcephei commented Oct 5, 2018

Copy link
Copy Markdown
Contributor Author

@mjsax Do you think this is likely the fix for the recent test failures?

It seems to work for me, but the absence of failure is no proof...

@mjsax mjsax added the streams label Oct 5, 2018
@mjsax

mjsax commented Oct 5, 2018

Copy link
Copy Markdown
Member

I assume it will work -- but frankly not 100% sure. Don't know exactly, how the broker advances it's mocked time internally.

@mjsax
mjsax requested a review from guozhangwang October 5, 2018 17:21
@vvcephei

vvcephei commented Oct 5, 2018

Copy link
Copy Markdown
Contributor Author

I also considered setting the broker's log retention to max_int hours, but I figured we should make the minimal change that we think will fix the problem (otherwise, we'll never learn what the actual fix was)

@guozhangwang

Copy link
Copy Markdown
Contributor

For Integration tests, the Embedded broker will not actively advance its MockTime unless MockTime.sleep() is called explicitly.

@guozhangwang guozhangwang left a comment

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.

The change lgtm, waiting for jenkins to pass

@vvcephei

vvcephei commented Oct 5, 2018

Copy link
Copy Markdown
Contributor Author

@guozhangwang @mjsax : the tests have passed.

@mjsax
mjsax merged commit eb8b1c0 into apache:trunk Oct 5, 2018
mjsax pushed a commit that referenced this pull request Oct 5, 2018
Reviewers: Guozhang Wang <guozhang@confluent.io>, Matthias J. Sax <matthias@confluent.io>
@mjsax

mjsax commented Oct 5, 2018

Copy link
Copy Markdown
Member

Merged to trunk and cherry-picked to 2.1 branch.

@vvcephei

vvcephei commented Oct 5, 2018

Copy link
Copy Markdown
Contributor Author

Thank you, @mjsax !

@vvcephei
vvcephei deleted the fix-suppress-integration-tests branch October 5, 2018 21:28
pengxiaolong pushed a commit to pengxiaolong/kafka that referenced this pull request Jun 14, 2019
Reviewers: Guozhang Wang <guozhang@confluent.io>, Matthias J. Sax <matthias@confluent.io>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants