Details

    • Type: Improvement Improvement
    • Status: Closed
    • Priority: Blocker Blocker
    • Resolution: Fixed
    • Affects Version/s: 1.1.0
    • Fix Version/s: 1.1.0
    • Component/s: None
    • Labels:
      None
    • Hadoop Flags:
      Incompatible change, Reviewed
    • Release Note:
      Hide
      This patch enables durable sync by default. Installation where HBase was not used, that used to run without setting "dfs.support.append" or setting it to false explicitly in the configuration, must add a new flag "dfs.durable.sync" and set it to false to preserve the previous semantics.
      Show
      This patch enables durable sync by default. Installation where HBase was not used, that used to run without setting "dfs.support.append" or setting it to false explicitly in the configuration, must add a new flag "dfs.durable.sync" and set it to false to preserve the previous semantics.

      Description

      Per HADOOP-8230 there's a request for a flag to disable the sync code paths that dfs.support.append used to enable. The sync method itself will still be available and have a broken implementation as that was the behavior before HADOOP-8230. This config flag should default to false as the primary motivation for HADOOP-8230 is so HBase works out-of-the-box with Hadoop 1.1.

      1. hadoop-8365.txt
        4 kB
        Eli Collins
      2. hadoop-8365.txt
        2 kB
        Eli Collins

        Issue Links

          Activity

          Hide
          Matt Foley added a comment -

          Closed upon release of Hadoop-1.1.0.

          Show
          Matt Foley added a comment - Closed upon release of Hadoop-1.1.0.
          Hide
          Todd Lipcon added a comment -

          I think you're misinterpreting HDFS-3731: the bug there is that data which was in-progress during the upgrade will be lost in the upgraded cluster. But without durable sync, all the blocks being written would have been lost anyway. That is to say, the bug makes durable sync just regress back to the old non-durable behavior. Anyone who is not using the feature would have lost the same amount of data.

          Or am I the one misinterpreting?

          Show
          Todd Lipcon added a comment - I think you're misinterpreting HDFS-3731 : the bug there is that data which was in-progress during the upgrade will be lost in the upgraded cluster. But without durable sync, all the blocks being written would have been lost anyway. That is to say, the bug makes durable sync just regress back to the old non-durable behavior. Anyone who is not using the feature would have lost the same amount of data. Or am I the one misinterpreting?
          Hide
          Suresh Srinivas added a comment -

          Look at HDFS-3731. 2.x upgrades does not handle this functionality well. For people who did not need durable sync, by turning the feature on by default, we are causing unnecessary upgrade issues.

          Show
          Suresh Srinivas added a comment - Look at HDFS-3731 . 2.x upgrades does not handle this functionality well. For people who did not need durable sync, by turning the feature on by default, we are causing unnecessary upgrade issues.
          Hide
          Eli Collins added a comment -

          Thanks for the review Suresh, I've committed this to branch-1 and merged to branch-1.1.

          Show
          Eli Collins added a comment - Thanks for the review Suresh, I've committed this to branch-1 and merged to branch-1.1.
          Hide
          Suresh Srinivas added a comment -

          +1 for the patch.

          Show
          Suresh Srinivas added a comment - +1 for the patch.
          Hide
          Eli Collins added a comment -

          Updated patch.

          1. Renames the config to dfs.durable.sync and flips the polarity, the previous name was to be consistent with allow.broken.append, and because if you disable this option you're breaking sync .

          2. Reintroduced this check around the BBW directory reporting. I left it out of the previous patch because BBW handling is independent of sync (even if you don't persist blocks you'll still have BBW).

          Show
          Eli Collins added a comment - Updated patch. 1. Renames the config to dfs.durable.sync and flips the polarity, the previous name was to be consistent with allow.broken.append, and because if you disable this option you're breaking sync . 2. Reintroduced this check around the BBW directory reporting. I left it out of the previous patch because BBW handling is independent of sync (even if you don't persist blocks you'll still have BBW).
          Hide
          Suresh Srinivas added a comment -

          Comments:

          1. The check added into FSNamesystem.java also needs to be added to DataNode.java, FSDataset.java, where earlier support for append was checked.
          2. Not sure why you are calling it broken sync. Can you remove broken from variable and configuration name.
          Show
          Suresh Srinivas added a comment - Comments: The check added into FSNamesystem.java also needs to be added to DataNode.java, FSDataset.java, where earlier support for append was checked. Not sure why you are calling it broken sync. Can you remove broken from variable and configuration name.
          Hide
          Eli Collins added a comment -

          Patch attached.

          Show
          Eli Collins added a comment - Patch attached.
          Hide
          Eli Collins added a comment -

          But sync() does not have a broken implementation as has been shown from its users

          Sync does not work on Hadoop 1.0 unless you enable dfs.support.append, which is why eg HBase requires it be set.

          Show
          Eli Collins added a comment - But sync() does not have a broken implementation as has been shown from its users Sync does not work on Hadoop 1.0 unless you enable dfs.support.append, which is why eg HBase requires it be set.
          Hide
          Harsh J added a comment -
          Show
          Harsh J added a comment - Typo: s/ HADOOP-8320 / HADOOP-8230
          Hide
          Harsh J added a comment -

          Eli,

          One clarification post the HADOOP-8320 ones, for this particular part in the description:

          The sync method itself will still be available and have a broken implementation as that was the behavior before HADOOP-8230.

          But sync() does not have a broken implementation as has been shown from its users. I do not understand that sentence completely, as HADOOP-8230 did not change any sync()-related code, so whatever was working or broken before is still the very same and will be even after what this issue aims to change.

          Show
          Harsh J added a comment - Eli, One clarification post the HADOOP-8320 ones, for this particular part in the description: The sync method itself will still be available and have a broken implementation as that was the behavior before HADOOP-8230 . But sync() does not have a broken implementation as has been shown from its users. I do not understand that sentence completely, as HADOOP-8230 did not change any sync()-related code, so whatever was working or broken before is still the very same and will be even after what this issue aims to change.
          Hide
          Suresh Srinivas added a comment -

          Marking this as blocker.

          Show
          Suresh Srinivas added a comment - Marking this as blocker.

            People

            • Assignee:
              Eli Collins
              Reporter:
              Eli Collins
            • Votes:
              0 Vote for this issue
              Watchers:
              8 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved:

                Development