Review Board 1.7.22


move LogCleanerManager to a separate file and improve unit test

Review Request #15674 - Created Nov. 19, 2013 and updated

Jun Rao
KAFKA-1074
Reviewers
kafka
kafka
move LogCleanerManager to a separate file and improve unit test


rename LogCleanerStates to LogCleanerManagers; optimize truncateTo to only pause cleaning if the truncated offset is not on the active segment


add test for aborting during cleaning


checkpoint recovery points in truncateFullyAndStartAt()


move cleaning states and locks to a separate class


minor fix 2


minor fix


remove unused var and exception


support pause/resume log clean and remove OptimisticLockFailureException


kafka-1074; minor fix3


kafka-1074; minor fix2


kafka-1074; minor fix


kafka-1074; better synchronization with log cleaner


kafka-1074; fix 4


kafka-1074; fix 3


kafka-1074; fix 2


kafka-1074

 
Total:
20
Open:
8
Resolved:
11
Dropped:
1
Status:
From:
Description From Last Updated Status
Are we going to have other checkDone implementations for this case? If not we could just save passing it as ... Guozhang Wang Dec. 5, 2013, 1:53 a.m. Open
Where should this exception be captured? Guozhang Wang Dec. 5, 2013, 1:53 a.m. Open
Do we have to keep two buildOffsetMap functions here? Guozhang Wang Dec. 5, 2013, 1:53 a.m. Open
Is there still a risk that after waitUntilInProgressFlusherIsDone passed another flushing procedure starts? Guozhang Wang Dec. 5, 2013, 1:53 a.m. Open
Another alternative would just be to let the background flush fail and log it rather than shutting down--it is after ... Jay Kreps Dec. 10, 2013, 1:27 a.m. Open
It seems like the logic for chosing logs is not part of the state management, is there any way to ... Jay Kreps Jan. 3, 2014, 4:22 p.m. Open
the method description says it should block until the cleaner has processed upto the given offset. But it doesn't look ... Neha Narkhede Jan. 6, 2014, 9:18 p.m. Open
Is this change intended? Neha Narkhede Jan. 7, 2014, 11:41 p.m. Open
Review request changed
Updated (Jan. 7, 2014, 6:51 p.m.)
  • rename LogCleanerStates to LogCleanerManagers; optimize truncateTo to only pause cleaning if the truncated offset is not on the active segment
    
    
    add test for aborting during cleaning
    
    
    checkpoint recovery points in truncateFullyAndStartAt()
    
    
    move cleaning states and locks to a separate class
    
    
    minor fix 2
    
    
    minor fix
    
    
    remove unused var and exception
    
    
    support pause/resume log clean and remove OptimisticLockFailureException
    
    
    kafka-1074; minor fix3
    
    
    kafka-1074; minor fix2
    
    
    kafka-1074; minor fix
    
    
    kafka-1074; better synchronization with log cleaner
    
    
    kafka-1074; fix 4
    
    
    kafka-1074; fix 3
    
    
    kafka-1074; fix 2
    
    
    kafka-1074

    move LogCleanerManager to a separate file and improve unit test
    
    
    rename LogCleanerStates to LogCleanerManagers; optimize truncateTo to only pause cleaning if the truncated offset is not on the active segment
    
    
    add test for aborting during cleaning
    
    
    checkpoint recovery points in truncateFullyAndStartAt()
    
    
    move cleaning states and locks to a separate class
    
    
    minor fix 2
    
    
    minor fix
    
    
    remove unused var and exception
    
    
    support pause/resume log clean and remove OptimisticLockFailureException
    
    
    kafka-1074; minor fix3
    
    
    kafka-1074; minor fix2
    
    
    kafka-1074; minor fix
    
    
    kafka-1074; better synchronization with log cleaner
    
    
    kafka-1074; fix 4
    
    
    kafka-1074; fix 3
    
    
    kafka-1074; fix 2
    
    
    kafka-1074
  • changed from rename LogCleanerStates to LogCleanerManagers; optimize truncateTo to only pause cleaning if the truncated offset is not on the active segment to move LogCleanerManager to a separate file and improve unit test
Ship it!
Posted (Jan. 7, 2014, 11:42 p.m.)
Other than the question below, I think this patch looks good.
Is this change intended?
  1. This is a diff btw v6 and v7, but is not in v7 itself.