Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

KAFKA-5752: Update timeIndex, txnIndex file pointers to renamed (to be deleted) files #3700

Closed
wants to merge 2 commits into from
Closed

KAFKA-5752: Update timeIndex, txnIndex file pointers to renamed (to be deleted) files #3700

wants to merge 2 commits into from

Conversation

omkreddy
Copy link
Contributor

No description provided.

@omkreddy
Copy link
Contributor Author

didn't find any easy way to test this. pl let me know if you have any ideas

ijuma
ijuma reviewed Aug 19, 2017
View reviewed changes
Copy link
Contributor

@ijuma ijuma left a comment

Choose a reason for hiding this comment

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

Thanks for the PR. With regards to the test, asyncDelete could return the removed log and we can inspect that the segments and log all point to the renamed directory.

@ijuma
Copy link
Contributor

ijuma commented Aug 19, 2017

We should probably also add a test that deletes a topic and adds it back as this tends to uncover issues.

@omkreddy
Copy link
Contributor Author

@ijuma added a simple test to check the file pointers.
We already have a "DeleteTopicTest.testRecreateTopicAfterDeletion " test. not sure this helps us to uncover these kinds of issues.

@ijuma
Copy link
Contributor

ijuma commented Aug 22, 2017

@omkreddy thanks for the update. A few suggested improvements:

ijuma@193a0c0

If you agree, please include them in your PR.

@omkreddy
Copy link
Contributor Author

@ijuma Thanks for the improvements. Updated the PR with your changes.

ijuma
ijuma approved these changes Aug 22, 2017
View reviewed changes
Copy link
Contributor

@ijuma ijuma left a comment

Choose a reason for hiding this comment

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

Thanks, LGTM. Will merge to trunk and 0.11.0 branches after the Jenkins builds complete.

@asfgit asfgit closed this in 3615ef7 Aug 22, 2017
asfgit pushed a commit that referenced this pull request Aug 22, 2017
@omkreddy @ijuma
KAFKA-5752; Update index files correctly during async delete
fa2751e 
timeIndex and txnIndex were not being updated previously.

Author: Manikumar Reddy <manikumar.reddy@gmail.com>

Reviewers: Ismael Juma <ismael@juma.me.uk>

Closes #3700 from omkreddy/KAFKA-5752

(cherry picked from commit 3615ef7)
Signed-off-by: Ismael Juma <ismael@juma.me.uk>
@omkreddy omkreddy deleted the KAFKA-5752 branch July 3, 2018 15:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@ijuma ijuma ijuma approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@omkreddy @ijuma