Description
While reviewing the patch for OAK-1582 I stumbled over a few issues with MarkSweepGarbageCollector that need improving. First an foremost MarkSweepGarbageCollector needs better documentation. The current javadoc as for many methods and arguments their semantics and invariants are unclear.
Furthermore:
- MarkSweepGarbageCollector#init(): why an init method, and not pass the respective arguments directly to the constructor? Also at when are clients allowed to call init? Can I call it while a a GC cycle is currently taking place?
- Is there (do we need) a protection for multiple GCs being initiated in parallel?
- MarkSweepGarbageCollector.Sweeper#run and MarkSweepGarbageCollector.BlobIdRetriever#retrieve catch Exception and e.printStackTrace(). This needs improving.
- MarkSweepGarbageCollector#sweep catches InterruptedExceptionInterruptedException and {{e.printStackTrace()}. This is wrong as at least the threads interrupted status need to be set.
- DocumentNodeStore#getReferencedBlobsIterator is passed into MarkSweepGarbageCollector#init) in DocumentNodeStoreService. Won't this iterator be consumed after the first gc run such that any further run won't do anything?
amitj_76, could you have a look at these points and create sub tasks as required?
Fixed this with http://svn.apache.org/r1583017. Currently a single MarkSweepGarbageCollector is created an used. However I am not sure if MarkSweepGarbageCollector is thread safe as it has instance state. Probably better way would be to create a new instance for every invocation