Skip to content

deleting *.tmp WAL files on startup#10317

Merged
codesome merged 4 commits into
prometheus:mainfrom
Howie59:fix-10245
Mar 24, 2022
Merged

deleting *.tmp WAL files on startup#10317
codesome merged 4 commits into
prometheus:mainfrom
Howie59:fix-10245

Conversation

@Howie59

@Howie59 Howie59 commented Feb 20, 2022

Copy link
Copy Markdown
Contributor

Signed-off-by: lihaowei [email protected]

Resolves #10245

@Howie59
fix issue prometheus#10245
a57218f 
Signed-off-by: lihaowei <[email protected]>
@Howie59
Howie59 requested a review from codesome as a code owner February 20, 2022 09:02
codesome
codesome reviewed Feb 24, 2022
View reviewed changes
Comment thread tsdb/db.go Outdated
Comment thread tsdb/db_test.go Outdated
Comment thread tsdb/db.go
@Howie59
minor changes
4d17f99 
Signed-off-by: lihaowei <[email protected]>
codesome
codesome reviewed Mar 2, 2022
View reviewed changes

@codesome codesome left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Another suggestion: isTmpBlockDir -> isTmpDir

Comment thread tsdb/db_test.go Outdated
@Howie59

Howie59 commented Mar 10, 2022

Copy link
Copy Markdown
Contributor Author

@codesome PTAL

harkishen
harkishen suggested changes Mar 11, 2022
View reviewed changes
Comment thread tsdb/db.go Outdated
Comment thread tsdb/db.go Outdated
@harkishen

harkishen commented Mar 11, 2022

Copy link
Copy Markdown
Contributor

I think the issue also involves cleaning up meta.json.tmp file which is not being cleaned here. Hence, the PR description should not say resolves as that will close the issue upon merge.

codesome
codesome approved these changes Mar 15, 2022
View reviewed changes

@codesome codesome left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

LGTM. Can you please fix the DCO? I will merge after that

@codesome

codesome commented Mar 15, 2022

Copy link
Copy Markdown
Member

I think the issue also involves cleaning up meta.json.tmp file which is not being cleaned here. Hence, the PR description should not say resolves as that will close the issue upon merge.

I see that the issue is titled (and describes) deleting checkpoint dirs, so it should be fine to close it. (the meta.json.tmp file stuff is in code comment, which is still intact)

Howie59 added 2 commits March 16, 2022 08:00
@Howie59
review changes
365f48e 
Signed-off-by: lihaowei <[email protected]>
@Howie59
minor changes
4cc751b 
Signed-off-by: lihaowei <[email protected]>
@Howie59

Howie59 commented Mar 16, 2022

Copy link
Copy Markdown
Contributor Author

LGTM. Can you please fix the DCO? I will merge after that

done

@codesome
codesome merged commit 1291ec7 into prometheus:main Mar 24, 2022
@Howie59
Howie59 deleted the fix-10245 branch March 24, 2022 10:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@codesome codesome codesome approved these changes
+1 more reviewer
@harkishen harkishen harkishen approved these changes
Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Proposal: Clean up leftover checkpoint tmp directories during startup

3 participants

@Howie59 @harkishen @codesome