Kafka
  1. Kafka
  2. KAFKA-1074

Reassign partitions should delete the old replicas from disk

    Details

    • Type: Bug Bug
    • Status: Resolved
    • Priority: Major Major
    • Resolution: Fixed
    • Affects Version/s: 0.8.0
    • Fix Version/s: 0.8.1
    • Component/s: core
    • Labels:
      None

      Description

      Currently, after reassigning replicas to other brokers, the old replicas are not removed from disk and have to be deleted manually.

      1. KAFKA-1074.patch
        12 kB
        Jun Rao
      2. KAFKA-1074_2013-12-04_10:14:13.patch
        30 kB
        Jun Rao
      3. KAFKA-1074_2013-12-16_09:43:53.patch
        42 kB
        Jun Rao
      4. KAFKA-1074_2014-01-02_08:36:37.patch
        49 kB
        Jun Rao
      5. KAFKA-1074_2014-01-03_12:30:47.patch
        50 kB
        Jun Rao
      6. KAFKA-1074_2014-01-04_08:40:55.patch
        50 kB
        Jun Rao
      7. KAFKA-1074_2014-01-07_10:55:08.patch
        51 kB
        Jun Rao

        Issue Links

          Activity

          Hide
          Jun Rao added a comment -

          Removing the old replica log from the disk itself is simple. What we need to make sure is that all potential outstanding operations on the deleted log are handled properly. In particular, we don't want any potential IOException due to the missing log causes the broker to halt.

          1. All read operations are ok since we already handle unexpected exceptions in KafkaApi.

          2. Writing to the log by the producer request, the replica fetcher or the log flusher: We need to make sure that after the log is deleted, no more writes/flushes will be attempted on the log. This can be achieved by:
          2.1 For producer requests, the delete partition operation will synchronize on the leaderAndIsrUpdate lock.
          2.2 For replica fetcher, this is already handled since the fetcher is removed before the log is deleted.
          2.3 For log flusher, the flush and the delete will now synchronize on a delete lock.

          Show
          Jun Rao added a comment - Removing the old replica log from the disk itself is simple. What we need to make sure is that all potential outstanding operations on the deleted log are handled properly. In particular, we don't want any potential IOException due to the missing log causes the broker to halt. 1. All read operations are ok since we already handle unexpected exceptions in KafkaApi. 2. Writing to the log by the producer request, the replica fetcher or the log flusher: We need to make sure that after the log is deleted, no more writes/flushes will be attempted on the log. This can be achieved by: 2.1 For producer requests, the delete partition operation will synchronize on the leaderAndIsrUpdate lock. 2.2 For replica fetcher, this is already handled since the fetcher is removed before the log is deleted. 2.3 For log flusher, the flush and the delete will now synchronize on a delete lock.
          Hide
          Jun Rao added a comment -

          Created reviewboard https://reviews.apache.org/r/15674/
          against branch origin/trunk

          Show
          Jun Rao added a comment - Created reviewboard https://reviews.apache.org/r/15674/ against branch origin/trunk
          Hide
          Jun Rao added a comment -

          Updated reviewboard https://reviews.apache.org/r/15674/
          against branch origin/trunk

          Show
          Jun Rao added a comment - Updated reviewboard https://reviews.apache.org/r/15674/ against branch origin/trunk
          Hide
          Jun Rao added a comment -

          Summary of the new patch:
          LogCleaner:
          *added the logic to cancel an in-progress cleaning task
          *made the cleaner thread a shutdownable, but not interruptible thread. Interrupting the cleaner may cause the file channel to be closed, which will fail other operations like log flushing during shutdown.

          LogManager:

          • added the logic to wait until an in-progress flushing process complete
          • to delete a log do
            (1) take the log to be deleted from log lists so that log cleaner and log flusher won't see it in the future
            (2) cancel any in-progress cleaning task
            (3) wait until any in-progress flushing process to complete
            (4) at this moment, we know the log won't be touched by the cleaner or the flusher any more and we can delete the whole directory synchronously.

          Todos:

          • If the overall logic looks good, we can get rid of OptimisticLockFailureException in LogCleaner too by canceling any cleaner task before truncating the log. This can be done in a follow up jira.
          Show
          Jun Rao added a comment - Summary of the new patch: LogCleaner: *added the logic to cancel an in-progress cleaning task *made the cleaner thread a shutdownable, but not interruptible thread. Interrupting the cleaner may cause the file channel to be closed, which will fail other operations like log flushing during shutdown. LogManager: added the logic to wait until an in-progress flushing process complete to delete a log do (1) take the log to be deleted from log lists so that log cleaner and log flusher won't see it in the future (2) cancel any in-progress cleaning task (3) wait until any in-progress flushing process to complete (4) at this moment, we know the log won't be touched by the cleaner or the flusher any more and we can delete the whole directory synchronously. Todos: If the overall logic looks good, we can get rid of OptimisticLockFailureException in LogCleaner too by canceling any cleaner task before truncating the log. This can be done in a follow up jira.
          Hide
          Jun Rao added a comment -

          Updated reviewboard https://reviews.apache.org/r/15674/
          against branch origin/trunk

          Show
          Jun Rao added a comment - Updated reviewboard https://reviews.apache.org/r/15674/ against branch origin/trunk
          Hide
          Jun Rao added a comment -

          Summary of changes since the previous patch.

          LogCleaner:

          • Added the logic to pause and resume the cleaning of a log. Canceling the cleaning is implemented as pausing, followed by resuming.

          LogManager:

          • Don't halt when background flush hits an IOException since the same IOException will be hit during log append, which will halt the broker. This removes the need to synchronize btw the flushing and the deleting of the log.
          • Removed OptimisticLockFailureException in LogCleaner. When a log needs to be truncated, first pause log cleaning, then truncate the log, and finally resume log cleaning.
          Show
          Jun Rao added a comment - Summary of changes since the previous patch. LogCleaner: Added the logic to pause and resume the cleaning of a log. Canceling the cleaning is implemented as pausing, followed by resuming. LogManager: Don't halt when background flush hits an IOException since the same IOException will be hit during log append, which will halt the broker. This removes the need to synchronize btw the flushing and the deleting of the log. Removed OptimisticLockFailureException in LogCleaner. When a log needs to be truncated, first pause log cleaning, then truncate the log, and finally resume log cleaning.
          Hide
          Jun Rao added a comment -

          Updated reviewboard https://reviews.apache.org/r/15674/
          against branch origin/trunk

          Show
          Jun Rao added a comment - Updated reviewboard https://reviews.apache.org/r/15674/ against branch origin/trunk
          Hide
          Jun Rao added a comment -

          Summary of changes since the previous patch.
          LogCleaner:

          • moved all variables and locks associated with state management into a separate class LogCleanerStates.
          • removed the semaphore for unit test and used a simpler approach to just keep retrying every 10ms. Will file a separate jira for improving the unit test.
            Log:
          • limited the scope of a few methods to log.
            LogManager:
          • forced checkpointing recovery points in truncateFullyAndStartAt() as well.
          Show
          Jun Rao added a comment - Summary of changes since the previous patch. LogCleaner: moved all variables and locks associated with state management into a separate class LogCleanerStates. removed the semaphore for unit test and used a simpler approach to just keep retrying every 10ms. Will file a separate jira for improving the unit test. Log: limited the scope of a few methods to log. LogManager: forced checkpointing recovery points in truncateFullyAndStartAt() as well.
          Hide
          Jun Rao added a comment -

          Updated reviewboard https://reviews.apache.org/r/15674/
          against branch origin/trunk

          Show
          Jun Rao added a comment - Updated reviewboard https://reviews.apache.org/r/15674/ against branch origin/trunk
          Hide
          Jun Rao added a comment -

          Updated reviewboard https://reviews.apache.org/r/15674/
          against branch origin/trunk

          Show
          Jun Rao added a comment - Updated reviewboard https://reviews.apache.org/r/15674/ against branch origin/trunk
          Hide
          Jun Rao added a comment -

          Updated reviewboard https://reviews.apache.org/r/15674/
          against branch origin/trunk

          Show
          Jun Rao added a comment - Updated reviewboard https://reviews.apache.org/r/15674/ against branch origin/trunk
          Hide
          Jun Rao added a comment -

          Thanks for the review. Committed to trunk.

          File a followup jira KAFKA-1201 to address unit test issue for LogCleanTest.

          Show
          Jun Rao added a comment - Thanks for the review. Committed to trunk. File a followup jira KAFKA-1201 to address unit test issue for LogCleanTest.

            People

            • Assignee:
              Jun Rao
              Reporter:
              Jun Rao
            • Votes:
              0 Vote for this issue
              Watchers:
              2 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved:

                Development