Comcast / sirius

A distributed system library for managing application reference data
http://comcast.github.io/sirius/
Apache License 2.0
298 stars 49 forks source link

Purge old DELETEs during live compaction #113

Closed weggert closed 9 years ago

weggert commented 9 years ago

This change is to address Issue 112, 'Old deletes need to be purged during Compaction'.

It adds the ability to specify a sirius.log.compaction-max-delete-age-hours configuration property to cause old Delete events to be purged during live compaction.

The configuration property defaults to a very large value, so by default Delete events will not be purged.

It also adds an option to WalTool to purge old Deletes, along with other options for old PUTs.

weggert commented 9 years ago

Pull request https://github.com/Comcast/sirius/pull/113 addresses this issue.

weggert commented 9 years ago

I added documentation to the purge deletes config item, and removed the unnecessary variable from WalTool.

clinedome-work commented 9 years ago

All in all, this looks great. A few minor comments, but with those addressed, I'd be +1. Thanks @weggert!

weggert commented 9 years ago

Per the feedback I pushed up changes to make the SegmentedCompactor private, remove the unnecessary comment, and simplify the Delete purge logic in the SegmentedCompactor.

clinedome-work commented 9 years ago

Awesome. Looks great. :+1: Gonna try and get another set of eyes to :+1: this then it's ready for a squash and merge.

tbarker9comcast commented 9 years ago

Had some minor comments, but once they are addressed this has my :+1:. This looks great to me.

weggert commented 9 years ago

I fixed the typo in the WalTool documentation, and I added a new test for the SegmentedCompactor Delete purge logic to make sure it works as expected when the max-time is bigger than the current time.

There is also a minor change for when getting the property to change siriusConfig.getProp(...) to siriusConfig.getInt(...) to avoid ClassCastExceptions if the property value is set with a Long.

weggert commented 9 years ago

Thanks for the reviews. :-) I squashed the updates into the original commit

clinedome-work commented 9 years ago

:+1: