Details

    • Type: Sub-task Sub-task
    • Status: Resolved
    • Priority: Minor Minor
    • Resolution: Fixed
    • Affects Version/s: None
    • Fix Version/s: 1.6.0
    • Component/s: client, master, tserver
    • Labels:
      None

      Description

      The existing CheckForMetadataProblems only checks the metadata table. It should also check the root table, now that those are split up.

      1. ACCUMULO-2291.patch
        5 kB
        John McNamee
      2. ACCUMULO-2291.v2.patch
        8 kB
        John McNamee

        Issue Links

          Activity

          Hide
          Josh Elser added a comment -

          John McNamee, fyi, for versions that have not yet been released, we don't mark an affects version.

          Show
          Josh Elser added a comment - John McNamee , fyi, for versions that have not yet been released, we don't mark an affects version.
          Hide
          Christopher Tubbs added a comment -

          John McNamee: I think the fix option is probably too poorly defined. It's probably good that you excluded the root table from being modified if there's a problem, but I don't know if we want to keep that option at all. Perhaps it should be removed, because it's too dangerous at all to include an autofix feature in that utility. If it's too dangerous for the root table, it's probably too dangerous for the metadata table also.

          Show
          Christopher Tubbs added a comment - John McNamee : I think the fix option is probably too poorly defined. It's probably good that you excluded the root table from being modified if there's a problem, but I don't know if we want to keep that option at all. Perhaps it should be removed, because it's too dangerous at all to include an autofix feature in that utility. If it's too dangerous for the root table, it's probably too dangerous for the metadata table also.
          Hide
          John McNamee added a comment -

          Removed the fix option.

          Show
          John McNamee added a comment - Removed the fix option.
          Hide
          Christopher Tubbs added a comment -

          Not a big deal, but in the future, it'd be best to leave the old attachment, and just add the new one. Something like:
          ACCUMULO-2291.v1.patch, ACCUMULO-2291.v2.patch, etc.

          Show
          Christopher Tubbs added a comment - Not a big deal, but in the future, it'd be best to leave the old attachment, and just add the new one. Something like: ACCUMULO-2291 .v1.patch, ACCUMULO-2291 .v2.patch, etc.
          Hide
          Josh Elser added a comment -

          in the future, it'd be best to leave the old attachment, and just add the new one

          Agree! Context is always appreciated. When recommendations/etc are made, it's quite easier to make sure they were all caught by looks at the changes between both.

          Show
          Josh Elser added a comment - in the future, it'd be best to leave the old attachment, and just add the new one Agree! Context is always appreciated. When recommendations/etc are made, it's quite easier to make sure they were all caught by looks at the changes between both.
          Hide
          John McNamee added a comment -

          @Josh @Christopher OK I will keep that in mind. Thanks for the feed back.

          Show
          John McNamee added a comment - @Josh @Christopher OK I will keep that in mind. Thanks for the feed back.
          Hide
          John Vines added a comment -

          I disagree with removing of the fix flag. This is a surgical tool for both diagnosing and repairing situations that can be entered from undiscovered bugs. This tools existence is for situations where things are broken and removing that functionality neuters it.

          Show
          John Vines added a comment - I disagree with removing of the fix flag. This is a surgical tool for both diagnosing and repairing situations that can be entered from undiscovered bugs. This tools existence is for situations where things are broken and removing that functionality neuters it.
          Hide
          Christopher Tubbs added a comment -

          John Vines The --fix option doesn't actually define what kinds of problems it can fix. It's very unpredictable in that way, from the user's perspective, and is dangerous to use. What it actually does is remove the loc... which can introduce multiple-assignment. I'd rather leave it out, until we have a better, more comprehensive tool for diagnosing and fixing problems that doesn't just try to do dangerous automagical things.

          Show
          Christopher Tubbs added a comment - John Vines The --fix option doesn't actually define what kinds of problems it can fix. It's very unpredictable in that way, from the user's perspective, and is dangerous to use. What it actually does is remove the loc... which can introduce multiple-assignment. I'd rather leave it out, until we have a better, more comprehensive tool for diagnosing and fixing problems that doesn't just try to do dangerous automagical things.
          Hide
          John McNamee added a comment -

          Fixed Formatting.

          Show
          John McNamee added a comment - Fixed Formatting.
          Hide
          John Vines added a comment -

          Then give it a different name, don't just remove it.

          Sent from my phone, please pardon the typos and brevity.

          Show
          John Vines added a comment - Then give it a different name, don't just remove it. Sent from my phone, please pardon the typos and brevity.
          Hide
          Christopher Tubbs added a comment -

          John Vines. The issue here is that we're in this situation where, in order for this tool to support the root table as well as the metadata table, we need to define how these additional options apply to the root table. One option is to leave the --fix option as is, and only apply to the metadata table, because applying to the root table is too dangerous and ill-advised. Another option is to extend the --fix option to attempt to fix the root table as well. The third option is to remove it entirely, so it will not work for either table, because it's too dangerous.

          Making this tool more comprehensive and useful, with more informed fixes to specific problems, is outside the scope of this ticket. John McNamee and I were only looking at how best to fit the root table in to satisfy this ticket.

          Show
          Christopher Tubbs added a comment - John Vines . The issue here is that we're in this situation where, in order for this tool to support the root table as well as the metadata table, we need to define how these additional options apply to the root table. One option is to leave the --fix option as is, and only apply to the metadata table, because applying to the root table is too dangerous and ill-advised. Another option is to extend the --fix option to attempt to fix the root table as well. The third option is to remove it entirely, so it will not work for either table, because it's too dangerous. Making this tool more comprehensive and useful, with more informed fixes to specific problems, is outside the scope of this ticket. John McNamee and I were only looking at how best to fit the root table in to satisfy this ticket.
          Hide
          Keith Turner added a comment -

          I was looking at the --fix option it makes alot of assumptions, but does not check for these assumptions. Looking at the tool I am not convinced that it would not misinterpret a split as a hole in the metatatable and "fix it" messing up the table. The tool should probably check the table is offline and that the tablet has not location before "fixing" a hole. Also the deletion of multiple locations seems arbitrary and dangerous, it should probably check in zookeeper and see what tservers are alive. If all are alive it probably should not delete any locations.

          Show
          Keith Turner added a comment - I was looking at the --fix option it makes alot of assumptions, but does not check for these assumptions. Looking at the tool I am not convinced that it would not misinterpret a split as a hole in the metatatable and "fix it" messing up the table. The tool should probably check the table is offline and that the tablet has not location before "fixing" a hole. Also the deletion of multiple locations seems arbitrary and dangerous, it should probably check in zookeeper and see what tservers are alive. If all are alive it probably should not delete any locations.
          Hide
          Eric Newton added a comment -

          This is a surgical tool

          No, it is not. It does no pre-checks to ensure tables are offline. It removes all but the first assignment which is arbitrary. It does not detect tablets in mid-split.

          Show
          Eric Newton added a comment - This is a surgical tool No, it is not. It does no pre-checks to ensure tables are offline. It removes all but the first assignment which is arbitrary. It does not detect tablets in mid-split.
          Hide
          Keith Turner added a comment -

          The --fix option has no test, no documentation, and seems very dangerous to actually use. Removing it seems like a really good option. The code will still be in git. If anyone wants this functionality then new ticket can be opened to do it properly (test, documentation, sanity checks).

          Show
          Keith Turner added a comment - The --fix option has no test, no documentation, and seems very dangerous to actually use. Removing it seems like a really good option. The code will still be in git. If anyone wants this functionality then new ticket can be opened to do it properly (test, documentation, sanity checks).
          Hide
          John Vines added a comment -

          In the few cases we've found issues with walog reply, it's been quite useful for getting the system back in order. But having it around in the history to use when needed should be sufficient.

          Show
          John Vines added a comment - In the few cases we've found issues with walog reply, it's been quite useful for getting the system back in order. But having it around in the history to use when needed should be sufficient.
          Hide
          ASF subversion and git services added a comment -

          Commit 0c4a956633f96a3426df94dcfefee4e86e5aa3f6 in accumulo's branch refs/heads/1.6.0-SNAPSHOT from John McNamee
          [ https://git-wip-us.apache.org/repos/asf?p=accumulo.git;h=0c4a956 ]

          ACCUMULO-2291 CheckForMetadataProblems now checks the root table as well as the metadata table for problems. Also removed the fix option for the metadata table.

          Signed-off-by: Christopher Tubbs <ctubbsii@apache.org>

          Show
          ASF subversion and git services added a comment - Commit 0c4a956633f96a3426df94dcfefee4e86e5aa3f6 in accumulo's branch refs/heads/1.6.0-SNAPSHOT from John McNamee [ https://git-wip-us.apache.org/repos/asf?p=accumulo.git;h=0c4a956 ] ACCUMULO-2291 CheckForMetadataProblems now checks the root table as well as the metadata table for problems. Also removed the fix option for the metadata table. Signed-off-by: Christopher Tubbs <ctubbsii@apache.org>
          Hide
          ASF subversion and git services added a comment -

          Commit 0c4a956633f96a3426df94dcfefee4e86e5aa3f6 in accumulo's branch refs/heads/master from John McNamee
          [ https://git-wip-us.apache.org/repos/asf?p=accumulo.git;h=0c4a956 ]

          ACCUMULO-2291 CheckForMetadataProblems now checks the root table as well as the metadata table for problems. Also removed the fix option for the metadata table.

          Signed-off-by: Christopher Tubbs <ctubbsii@apache.org>

          Show
          ASF subversion and git services added a comment - Commit 0c4a956633f96a3426df94dcfefee4e86e5aa3f6 in accumulo's branch refs/heads/master from John McNamee [ https://git-wip-us.apache.org/repos/asf?p=accumulo.git;h=0c4a956 ] ACCUMULO-2291 CheckForMetadataProblems now checks the root table as well as the metadata table for problems. Also removed the fix option for the metadata table. Signed-off-by: Christopher Tubbs <ctubbsii@apache.org>

            People

            • Assignee:
              John McNamee
              Reporter:
              Christopher Tubbs
            • Votes:
              0 Vote for this issue
              Watchers:
              7 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved:

                Development