-
Notifications
You must be signed in to change notification settings - Fork 13
4453 compaction creation batch files are never deleted #4599
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.
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
4453 compaction creation batch files are never deleted #4599
Conversation
...n-job-creation/src/main/java/sleeper/compaction/job/creation/AwsCompactionJobDispatcher.java
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Since the current AWS code for this does nothing, we could still merge these changes and it wouldn't cause a problem. To actually fix the bug, we still need to add the missing assertions and implement it against SQS.
If we want to merge this as is, we could split out a separate issue for just these changes, and link this PR to that instead. Right now this PR is linked to the actual bug issue so it'll close it if it's merged, and it isn't fixed yet.
...ion-core/src/test/java/sleeper/compaction/core/job/dispatch/CompactionJobDispatcherTest.java
Show resolved
Hide resolved
...ion-core/src/test/java/sleeper/compaction/core/job/dispatch/CompactionJobDispatcherTest.java
Show resolved
Hide resolved
a01d744
to
e787485
Compare
...ion-core/src/test/java/sleeper/compaction/core/job/dispatch/CompactionJobDispatcherTest.java
Outdated
Show resolved
Hide resolved
...paction-core/src/main/java/sleeper/compaction/core/job/dispatch/CompactionJobDispatcher.java
Outdated
Show resolved
Hide resolved
...ion-core/src/test/java/sleeper/compaction/core/job/dispatch/CompactionJobDispatcherTest.java
Show resolved
Hide resolved
...ion-core/src/test/java/sleeper/compaction/core/job/dispatch/CompactionJobDispatcherTest.java
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good, thanks.
Make sure you have checked all steps below.
Issue
PR"
Tests
Documentation
separate issue for that below.