Accumulo
  1. Accumulo
  2. ACCUMULO-1785

Alter config.sh to optionally just verify environment instead of making changes

    Details

    • Type: Improvement Improvement
    • Status: Resolved
    • Priority: Minor Minor
    • Resolution: Fixed
    • Affects Version/s: 1.4.4, 1.5.0
    • Fix Version/s: 1.4.5, 1.5.1, 1.6.0
    • Component/s: scripts
    • Labels:
      None

      Description

      Right now, the accumulo scripts presume that invocation always happens on a node in the cluster. This makes integrating with external cluster management software more difficult.

      The primary issue is that bin/config.sh expects to find a full cluster configuration directory at all times. In the case of a configuration directory that doesn't specify any hosts it takes steps to set up a standalone cluster. That effort is unneeded and undesirable if we're just e.g. trying to go through init from a non-participant host.

      Add an optional environment variable to have config.sh just verify the invariants needed for bin/accumulo to behave correctly. When present, don't make local directories or write to files.

      1. ACCUMULO-1785.2.patch.txt
        8 kB
        Sean Busbey
      2. ACCUMULO-1785.1.patch.txt
        8 kB
        Sean Busbey

        Issue Links

          Activity

          Sean Busbey created issue -
          Hide
          Sean Busbey added a comment -

          Patch against 1.4.5-SNAPSHOT for changes to implement ACCUMULO_VERIFY_ONLY for bin/config.sh.

          Can't get review board to work against the Accumulo project ATM. So I'll just put everything here.

          Testing done:

          • functional tests
          • starting w/o env variable set and no master/slaves correctly started in standalone mode.
          • starting w/env variable and no required variables and no master/slaves correctly complained and failed.
          • starting w/env variable and required variables and no master/slaves correctly init w/o writing crap, correctly failed to use start-all
          • starting with a normal cluster config

          One bit I'm not sure about: $

          {GC}

          and $

          {MONITOR}

          . Right now they're inferred if not present. If we're in VERIFY_ONLY mode, I'm not sure they're actually needed. AFAICT, they're only used in the bin/

          {start|stop}

          -* scripts.

          Any thoughts about pulling the entire checking/setting of them into the non-verify-only mode?

          Show
          Sean Busbey added a comment - Patch against 1.4.5-SNAPSHOT for changes to implement ACCUMULO_VERIFY_ONLY for bin/config.sh. Can't get review board to work against the Accumulo project ATM. So I'll just put everything here. Testing done: functional tests starting w/o env variable set and no master/slaves correctly started in standalone mode. starting w/env variable and no required variables and no master/slaves correctly complained and failed. starting w/env variable and required variables and no master/slaves correctly init w/o writing crap, correctly failed to use start-all starting with a normal cluster config One bit I'm not sure about: $ {GC} and $ {MONITOR} . Right now they're inferred if not present. If we're in VERIFY_ONLY mode, I'm not sure they're actually needed. AFAICT, they're only used in the bin/ {start|stop} -* scripts. Any thoughts about pulling the entire checking/setting of them into the non-verify-only mode?
          Sean Busbey made changes -
          Field Original Value New Value
          Attachment ACCUMULO-1785.1.patch.txt [ 12609063 ]
          Sean Busbey made changes -
          Status Open [ 1 ] Patch Available [ 10002 ]
          Hide
          Sean Busbey added a comment -
          Show
          Sean Busbey added a comment - Now with a review board posting
          Hide
          Sean Busbey added a comment -

          Attaching updated patch with results of review board feedback.

          Show
          Sean Busbey added a comment - Attaching updated patch with results of review board feedback.
          Sean Busbey made changes -
          Attachment ACCUMULO-1785.2.patch.txt [ 12612643 ]
          Hide
          Sean Busbey added a comment -

          Adding 1.6.0 as impacted version, since this will be excluded as an improvement

          Show
          Sean Busbey added a comment - Adding 1.6.0 as impacted version, since this will be excluded as an improvement
          Sean Busbey made changes -
          Affects Version/s 1.6.0 [ 12322468 ]
          Sean Busbey made changes -
          Assignee Sean Busbey [ busbey ]
          Hide
          Sean Busbey added a comment -

          Bump. Could a committer take a look at pushing this?

          Show
          Sean Busbey added a comment - Bump. Could a committer take a look at pushing this?
          Hide
          Bill Havanki added a comment -

          Re-bump! Looks like review is done, please commit? Thanks.

          Show
          Bill Havanki added a comment - Re-bump! Looks like review is done, please commit? Thanks.
          Hide
          Christopher Tubbs added a comment -

          What branch is this patch applicable to?

          Show
          Christopher Tubbs added a comment - What branch is this patch applicable to?
          Hide
          Sean Busbey added a comment -

          At the time, 1.4.5-SNAPSHOT. If it's giving problems, I can rebase.

          Show
          Sean Busbey added a comment - At the time, 1.4.5-SNAPSHOT. If it's giving problems, I can rebase.
          Hide
          Keith Turner added a comment -

          I am looking into applying this

          Show
          Keith Turner added a comment - I am looking into applying this
          Hide
          ASF subversion and git services added a comment -

          Commit a7d981b3759f16c094f7932a3cdc67e76813d85f in branch refs/heads/1.4.5-SNAPSHOT from Sean Busbey
          [ https://git-wip-us.apache.org/repos/asf?p=accumulo.git;h=a7d981b ]

          ACCUMULO-1785 adds ACCUMULO_VERIFY_ONLY mode to bin/config.sh.

          • When set, won't write to local filesystem
          • skips mkdir for ACCUMULO_LOG_DIR
          • skips standalone mode checks
          • skips writing to missing tracers file
          • Other behavior changes
          • if GC was set, we won't overwrite it
          • if MONITOR was set, we won't overwrite it

          Signed-off-by: Keith Turner <kturner@apache.org>

          Show
          ASF subversion and git services added a comment - Commit a7d981b3759f16c094f7932a3cdc67e76813d85f in branch refs/heads/1.4.5-SNAPSHOT from Sean Busbey [ https://git-wip-us.apache.org/repos/asf?p=accumulo.git;h=a7d981b ] ACCUMULO-1785 adds ACCUMULO_VERIFY_ONLY mode to bin/config.sh. When set, won't write to local filesystem skips mkdir for ACCUMULO_LOG_DIR skips standalone mode checks skips writing to missing tracers file Other behavior changes if GC was set, we won't overwrite it if MONITOR was set, we won't overwrite it Signed-off-by: Keith Turner <kturner@apache.org>
          Hide
          ASF subversion and git services added a comment -

          Commit a7d981b3759f16c094f7932a3cdc67e76813d85f in branch refs/heads/1.5.1-SNAPSHOT from Sean Busbey
          [ https://git-wip-us.apache.org/repos/asf?p=accumulo.git;h=a7d981b ]

          ACCUMULO-1785 adds ACCUMULO_VERIFY_ONLY mode to bin/config.sh.

          • When set, won't write to local filesystem
          • skips mkdir for ACCUMULO_LOG_DIR
          • skips standalone mode checks
          • skips writing to missing tracers file
          • Other behavior changes
          • if GC was set, we won't overwrite it
          • if MONITOR was set, we won't overwrite it

          Signed-off-by: Keith Turner <kturner@apache.org>

          Show
          ASF subversion and git services added a comment - Commit a7d981b3759f16c094f7932a3cdc67e76813d85f in branch refs/heads/1.5.1-SNAPSHOT from Sean Busbey [ https://git-wip-us.apache.org/repos/asf?p=accumulo.git;h=a7d981b ] ACCUMULO-1785 adds ACCUMULO_VERIFY_ONLY mode to bin/config.sh. When set, won't write to local filesystem skips mkdir for ACCUMULO_LOG_DIR skips standalone mode checks skips writing to missing tracers file Other behavior changes if GC was set, we won't overwrite it if MONITOR was set, we won't overwrite it Signed-off-by: Keith Turner <kturner@apache.org>
          Hide
          ASF subversion and git services added a comment -

          Commit a7d981b3759f16c094f7932a3cdc67e76813d85f in branch refs/heads/1.6.0-SNAPSHOT from Sean Busbey
          [ https://git-wip-us.apache.org/repos/asf?p=accumulo.git;h=a7d981b ]

          ACCUMULO-1785 adds ACCUMULO_VERIFY_ONLY mode to bin/config.sh.

          • When set, won't write to local filesystem
          • skips mkdir for ACCUMULO_LOG_DIR
          • skips standalone mode checks
          • skips writing to missing tracers file
          • Other behavior changes
          • if GC was set, we won't overwrite it
          • if MONITOR was set, we won't overwrite it

          Signed-off-by: Keith Turner <kturner@apache.org>

          Show
          ASF subversion and git services added a comment - Commit a7d981b3759f16c094f7932a3cdc67e76813d85f in branch refs/heads/1.6.0-SNAPSHOT from Sean Busbey [ https://git-wip-us.apache.org/repos/asf?p=accumulo.git;h=a7d981b ] ACCUMULO-1785 adds ACCUMULO_VERIFY_ONLY mode to bin/config.sh. When set, won't write to local filesystem skips mkdir for ACCUMULO_LOG_DIR skips standalone mode checks skips writing to missing tracers file Other behavior changes if GC was set, we won't overwrite it if MONITOR was set, we won't overwrite it Signed-off-by: Keith Turner <kturner@apache.org>
          Hide
          Keith Turner added a comment -

          Sean Busbey I applied the patch. There were merge conflicts when applying to 1.4.5-SNAPSHOT which I resolved. When I merged it into 1.5.1-SNAPSHOT there were also merge conflicts which I resolved. It merged cleanly from 1.5 to 1.6. Can you check the 1.4 and 1.5 conflict resolution?

          Show
          Keith Turner added a comment - Sean Busbey I applied the patch. There were merge conflicts when applying to 1.4.5-SNAPSHOT which I resolved. When I merged it into 1.5.1-SNAPSHOT there were also merge conflicts which I resolved. It merged cleanly from 1.5 to 1.6. Can you check the 1.4 and 1.5 conflict resolution?
          Hide
          Keith Turner added a comment -

          I noticed that I botched the comments at the top of the script during the merge. There are now two "Values always set by script" sections in 1.4.

          Show
          Keith Turner added a comment - I noticed that I botched the comments at the top of the script during the merge. There are now two "Values always set by script" sections in 1.4.
          Hide
          ASF subversion and git services added a comment -

          Commit b6931683a5bab1bfba9498c83451bb3daf0a8c25 in branch refs/heads/1.4.5-SNAPSHOT from [~keith_turner]
          [ https://git-wip-us.apache.org/repos/asf?p=accumulo.git;h=b693168 ]

          ACCUMULO-1785 comment was messged up when resolving patch conflict

          Show
          ASF subversion and git services added a comment - Commit b6931683a5bab1bfba9498c83451bb3daf0a8c25 in branch refs/heads/1.4.5-SNAPSHOT from [~keith_turner] [ https://git-wip-us.apache.org/repos/asf?p=accumulo.git;h=b693168 ] ACCUMULO-1785 comment was messged up when resolving patch conflict
          Hide
          ASF subversion and git services added a comment -

          Commit b6931683a5bab1bfba9498c83451bb3daf0a8c25 in branch refs/heads/1.5.1-SNAPSHOT from [~keith_turner]
          [ https://git-wip-us.apache.org/repos/asf?p=accumulo.git;h=b693168 ]

          ACCUMULO-1785 comment was messged up when resolving patch conflict

          Show
          ASF subversion and git services added a comment - Commit b6931683a5bab1bfba9498c83451bb3daf0a8c25 in branch refs/heads/1.5.1-SNAPSHOT from [~keith_turner] [ https://git-wip-us.apache.org/repos/asf?p=accumulo.git;h=b693168 ] ACCUMULO-1785 comment was messged up when resolving patch conflict
          Hide
          ASF subversion and git services added a comment -

          Commit b6931683a5bab1bfba9498c83451bb3daf0a8c25 in branch refs/heads/1.6.0-SNAPSHOT from [~keith_turner]
          [ https://git-wip-us.apache.org/repos/asf?p=accumulo.git;h=b693168 ]

          ACCUMULO-1785 comment was messged up when resolving patch conflict

          Show
          ASF subversion and git services added a comment - Commit b6931683a5bab1bfba9498c83451bb3daf0a8c25 in branch refs/heads/1.6.0-SNAPSHOT from [~keith_turner] [ https://git-wip-us.apache.org/repos/asf?p=accumulo.git;h=b693168 ] ACCUMULO-1785 comment was messged up when resolving patch conflict
          Hide
          Sean Busbey added a comment -

          The merges to 1.4.5-SNAPSHOT and 1.5.1-SNAPSHOT look fine, though as mentioned in ACCUMULO-1901 the behavior of GC stuff changed. I'll put a note there about it.

          Show
          Sean Busbey added a comment - The merges to 1.4.5-SNAPSHOT and 1.5.1-SNAPSHOT look fine, though as mentioned in ACCUMULO-1901 the behavior of GC stuff changed. I'll put a note there about it.
          Sean Busbey made changes -
          Status Patch Available [ 10002 ] Resolved [ 5 ]
          Fix Version/s 1.4.5 [ 12324754 ]
          Fix Version/s 1.5.1 [ 12324399 ]
          Fix Version/s 1.6.0 [ 12322468 ]
          Resolution Fixed [ 1 ]
          Hide
          Josh Elser added a comment -

          I just noticed that these changes also impacted how ACCUMULO_HOME is used. I'm not sure if it was intentional or not.

          Before these changes, bin/config.sh always calculated ACCUMULO_HOME based on the location of itself (config.sh). Since the changes for this issue, ACCUMULO_HOME is only calculated, when ACCUMULO_HOME is not already set or is set to the empty string.

          From an error-catching perspective, should we add something that does some validation of the provided ACCUMULO_HOME when we don't calculate it? I can easily see cases where the user mistypes an export of ACCUMULO_HOME or programmatically generates an incorrect path and suddenly none of the classpaths work.

          Show
          Josh Elser added a comment - I just noticed that these changes also impacted how ACCUMULO_HOME is used. I'm not sure if it was intentional or not. Before these changes, bin/config.sh always calculated ACCUMULO_HOME based on the location of itself (config.sh). Since the changes for this issue, ACCUMULO_HOME is only calculated, when ACCUMULO_HOME is not already set or is set to the empty string. From an error-catching perspective, should we add something that does some validation of the provided ACCUMULO_HOME when we don't calculate it? I can easily see cases where the user mistypes an export of ACCUMULO_HOME or programmatically generates an incorrect path and suddenly none of the classpaths work.
          Hide
          Josh Elser added a comment -

          Reopening for discussion on (unintended) implications of changes.

          Show
          Josh Elser added a comment - Reopening for discussion on (unintended) implications of changes.
          Josh Elser made changes -
          Resolution Fixed [ 1 ]
          Status Resolved [ 5 ] Reopened [ 4 ]
          Hide
          Sean Busbey added a comment -

          That was an intentional change. Can we file a follow on ticket for the verification of a set ACCUMULO_HOME?

          Show
          Sean Busbey added a comment - That was an intentional change. Can we file a follow on ticket for the verification of a set ACCUMULO_HOME?
          Josh Elser made changes -
          Link This issue relates to ACCUMULO-1997 [ ACCUMULO-1997 ]
          Hide
          Josh Elser added a comment -

          That was an intentional change. Can we file a follow on ticket for the verification of a set ACCUMULO_HOME?

          Ok, I wasn't sure because things suddenly broke for me after this change, despite not setting ACCUMULO_VERIFY_ONLY. Opened ACCUMULO-1997.

          Show
          Josh Elser added a comment - That was an intentional change. Can we file a follow on ticket for the verification of a set ACCUMULO_HOME? Ok, I wasn't sure because things suddenly broke for me after this change, despite not setting ACCUMULO_VERIFY_ONLY. Opened ACCUMULO-1997 .
          Hide
          Josh Elser added a comment -

          Closing again for now – ACCUMULO-1997 will have necessary updates.

          Show
          Josh Elser added a comment - Closing again for now – ACCUMULO-1997 will have necessary updates.
          Josh Elser made changes -
          Status Reopened [ 4 ] Resolved [ 5 ]
          Resolution Fixed [ 1 ]
          Josh Elser made changes -
          Link This issue breaks ACCUMULO-2329 [ ACCUMULO-2329 ]
          Josh Elser made changes -
          Link This issue breaks ACCUMULO-2153 [ ACCUMULO-2153 ]
          Christopher Tubbs made changes -
          Affects Version/s 1.6.0 [ 12322468 ]
          Transition Time In Source Status Execution Times Last Executer Last Execution Date
          Open Open Patch Available Patch Available
          2h 52m 1 Sean Busbey 18/Oct/13 00:54
          Patch Available Patch Available Resolved Resolved
          48d 18h 28m 1 Sean Busbey 05/Dec/13 18:22
          Resolved Resolved Reopened Reopened
          4d 4h 10m 1 Josh Elser 09/Dec/13 22:32
          Reopened Reopened Resolved Resolved
          22h 40m 1 Josh Elser 10/Dec/13 21:13

            People

            • Assignee:
              Sean Busbey
              Reporter:
              Sean Busbey
            • Votes:
              0 Vote for this issue
              Watchers:
              4 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved:

                Development