I'd suggest to document in more detail the consequences of (not) using --opportunistic-tries. In particular
- using it gives the possibility to move instances even if a node is locked by some other operation - not using it can result in better instance placement, if the nodes currently being blocked would be suitable as a target. On Fri, Feb 21, 2014 at 1:30 PM, Petr Pudlák <[email protected]> wrote: > > > > On Tue, Feb 18, 2014 at 3:39 PM, Hrvoje Ribicic <[email protected]> wrote: > >> To start off the introduction of oppportunistic locking during instance >> creation, this patch adds the options allowing the locking to be >> invoked. >> >> Signed-off-by: Hrvoje Ribicic <[email protected]> >> --- >> src/Ganeti/Constants.hs | 9 +++++++++ >> tools/move-instance | 26 ++++++++++++++++++++++++++ >> 2 files changed, 35 insertions(+) >> >> diff --git a/src/Ganeti/Constants.hs b/src/Ganeti/Constants.hs >> index 7f8cfd1..d44e1fd 100644 >> --- a/src/Ganeti/Constants.hs >> +++ b/src/Ganeti/Constants.hs >> @@ -3211,6 +3211,15 @@ iallocatorSearchPath = >> AutoConf.iallocatorSearchPath >> defaultIallocatorShortcut :: String >> defaultIallocatorShortcut = "." >> >> +-- * Opportunistic allocator usage >> + >> +-- | Time delay in seconds between repeated opportunistic instance >> creations. >> +-- Rather than failing with an informative error message if the >> opportunistic >> +-- creation cannot grab enough nodes, for some uses it is better to >> retry the >> +-- creation with an interval between attempts. This is a reasonable >> default. >> +defaultOpportunisticRetryInterval :: Int >> +defaultOpportunisticRetryInterval = 10 >> + >> -- * Node evacuation >> >> nodeEvacPri :: String >> diff --git a/tools/move-instance b/tools/move-instance >> index 7ff8c4b..aa5e83d 100755 >> --- a/tools/move-instance >> +++ b/tools/move-instance >> @@ -126,6 +126,19 @@ PARALLEL_OPT = \ >> dest="parallel", metavar="<number>", >> help="Number of instances to be moved simultaneously") >> >> +OPPORTUNISTIC_TRIES_OPT = \ >> + cli.cli_option("--opportunistic-tries", action="store", type="int", >> + default=0, dest="opportunistic_tries", >> metavar="<number>", >> + help="Number of opportunistic instance creation >> attempts" >> + " before a normal creation is performed") >> + >> +OPPORTUNISTIC_DELAY_OPT = \ >> + cli.cli_option("--opportunistic-delay", action="store", type="int", >> + default=constants.DEFAULT_OPPORTUNISTIC_RETRY_INTERVAL, >> + dest="opportunistic_delay", metavar="<number>", >> + help="The delay between successive opportunistic >> instance" >> + " creation attempts, in seconds") >> + >> >> class Error(Exception): >> """Generic error. >> @@ -800,6 +813,8 @@ def ParseOptions(): >> parser.add_option(DEST_DISK_TEMPLATE_OPT) >> parser.add_option(COMPRESS_OPT) >> parser.add_option(PARALLEL_OPT) >> + parser.add_option(OPPORTUNISTIC_TRIES_OPT) >> + parser.add_option(OPPORTUNISTIC_DELAY_OPT) >> >> (options, args) = parser.parse_args() >> >> @@ -831,6 +846,17 @@ def CheckOptions(parser, options, args): >> bool(options.dest_primary_node or options.dest_secondary_node)): >> parser.error("Destination node and iallocator options exclude each >> other") >> >> + if (not options.iallocator and (options.opportunistic_tries > 0)): >> + parser.error("Opportunistic instance creation can only be used with >> an" >> + " iallocator") >> + >> + if options.opportunistic_tries < 0: >> + parser.error("Number of opportunistic creation attempts must be >= >> 0") >> + >> + if options.opportunistic_delay <= 0: >> + parser.error("The delay between two successive creation attempts >> must be" >> + " greater than zero") >> + >> if len(instance_names) == 1: >> # Moving one instance only >> if options.hvparams: >> -- >> 1.9.0.rc1.175.g0b1dcb5 >> >> > I'd add a check that if a user specifies --opportunistic-delay but > not --opportunistic-tries, the command fails with an error. Otherwise the > user might get confused why the retries aren't happening. >
