7
\$\begingroup\$

I have created a batch script to start the services in a gap of 10 min. My batch file will start the service on the local system and then will wait for 10 mins and starts the service on another system and then another. I have used the timeout command with nobreak statement which does not allows the user to stop the waiting time. If any changes need to be done, please suggest something.

@echo off
set timeout=600
set host1=Server_IP
set host1_service_name=swiftalm
set host2=Server_IP
set host2_service_name=swiftalm
set host3=Server_IP
set host3_service_name=swiftalm
set host4=Server_IP
set host4_service_name=swiftalm

echo ******Starting the Digite service on host1*****.
sc \\%host1% start %host1_service_name%
timeout /t %timeout% /nobreak
echo ******Starting the Digite service on host2*****.
sc \\%host2% start %host2_service_name%
timeout /t %timeout% /nobreak
echo ******Starting the Digite service on host3*****.
sc \\%host3% start %host3_service_name%
timeout /t %timeout% /nobreak
echo ******Starting the Digite service on host4*****.
sc \\%host4% start %host4_service_name%
timeout /t %timeout% /nobreak
\$\endgroup\$
8
\$\begingroup\$

This is a nice and simple script. There's not a lot to mess up here. Really, any embellishments to the script are personal preference, and the suggestions I or anyone else might offer are little more than opinion here.

Be that as it may, I do have a few suggestions. As is my habit in all the other reviews I've offered thus far, I suggest a couple of general practice behavior changes.

  1. Add setlocal to the top of every script you write. This script uses a lot of variables. After you exit, those variables are still hanging around, junking up your environment, potentially causing problems for other scripts that expect them not to be defined. Adding setlocal just below @echo off will narrow the scope of those variables to this script, and the variables will be forgotten on exit (whether the script exits gracefully or not). See this page for a more detailed explanation of why this is considered good practice.

  2. Get into the habit whenever you set a variable to a string, to set "var=value" with the var + value pair enclosed in quotation marks. One of these days you may need to capture special characters like & or |, or some XML or HTML to a variable. If you set "var=<xml tags>" you don't have to worry about those special characters getting evaluated unintentionally. Get this habit under your fingers now and you will spend much less time in the future debugging.

    My script-specific suggestions may not be relevant, depending on your needs. Really, your script is mostly fine already, and I hope you don't consider these suggestions as my being hyper-critical, or suggesting you're doing anything wrong. Ultimately, you know what will work best for you.

  3. Unless you have a specific reason for including it, consider removing the final line, timeout /t %timeout% /nobreak. Does your script really need to pause for 10 minutes after executing its final action?

  4. Since the service name is the same for all four servers, how about doing set "service=swiftalm", then sc \\%host1% start %service% with %service% in all four sc commands? I see little reason to maintain four variables all with the same value.

  5. If the context under which this script runs has admin privilege on the remote servers, sc is lovely for starting services remotely. For future projects, if you ever find yourself needing to supply a username and password with the commands, wmic can perform the same task with different credentials.

    set /P "user=Domain\Username? "
    set /P "pass=Password? "
    wmic /node:%host1% /user:%user% /password:%pass% service where 'name="%service%"' call startservice
    

    If you wish, you can mask the input of %pass% with PowerShell (credit: Matt Williamson):

    <NUL set /P "=Password? "
    set "psCommand=powershell -command "$p=read-host -AsSecureString;^
    $m=[System.Runtime.InteropServices.Marshal];$m::PtrToStringAuto($m::SecureStringToBSTR($p))""
    for /f "usebackq delims=" %%p in (`%psCommand%`) do set "pass=%%p"
    
  6. You could condense your script quite a bit by adding a for /f loop to loop through your %hostN% variables.

    Example:

    @echo off
    setlocal
    set timeout=600
    set "service=swiftalm"
    set "host1=Server_IP1"
    set "host2=Server_IP2"
    set "host3=Server_IP3"
    set "host4=Server_IP4"
    
    for /f "tokens=2 delims==" %%I in ('set host') do (
    
        echo ******Starting the Digite service on %%~I*****.
        sc "\\%%~I" start %service%
    
        if not "%%~I"=="%host4%" timeout /t %timeout% /nobreak
    )
    

    ...although, admittedly, that may not as instantly readable as your code above. It's really a matter of personal preference.

  7. Instead of making the workstation from which you run this script perform the waiting, you could initiate a one-time scheduled task on each server, making the server do the waiting. Because this involves datetime math, it gets a little complicated. I typically borrow from JScript when I need to do date math.

    @if (@CodeSection == @Batch) @Then
    
    :: begin batch portion
    
    @echo off
    setlocal
    set "service=swiftalm"
    set "host1=Server_IP1"
    set "host2=Server_IP2"
    set "host3=Server_IP3"
    set "host4=Server_IP4"
    set /a timeout=600, loop = 1
    
    :loop
    echo;
    :: start first task immediately
    if %loop% equ 1 (
        echo ******Starting the Digite service on host%loop%******
        sc "\\%host1%" start %service%
        set /a loop += 1
        goto loop
    )
    :: else schedule for later remotely
    
    :: use JScript because cmd is horrible at date math.
    :: JScript passes stDate=date and stTime=time back to batch.
    for /f "delims=" %%I in ('cscript /nologo /e:Jscript "%~f0" %timeout% %loop%') do (
        set "%%I"
    )
    
    echo ******Scheduling the Digite service to start on host%loop%******
    setlocal enabledelayedexpansion
    schtasks /create /tn "Start %service%" /tr "cmd /c sc start %service%" /sc once /sd %stDate% /st %stTime% /s !host%loop%! /z
    endlocal
    
    :: If more hosts in the list, increment loop counter and go back
    set /a loop += 1
    if defined host%loop% goto loop
    
    :: Runtime complete.
    echo All tasks complete.  Press any key to exit.
    >NUL pause
    goto :EOF
    
    @end
    // end batch / begin JScript hybrid code
    
    var seconds = WSH.Arguments(0) * WSH.Arguments(1) - WSH.Arguments(0),
    
    future = new Date(new Date().getTime() + seconds * 1000),
    
    z = function(num) { return num < 10 ? '0' + num : num },
    
    MM = z(future.getMonth() + 1),
    DD = z(future.getDate()),
    YYYY = z(future.getFullYear()),
    
    oSH = WSH.CreateObject('wscript.shell'),
    localeDateFormat = oSH.RegRead('HKCU\\Control Panel\\International\\iDate'),
    localeDateSeparator = oSH.RegRead('HKCU\\Control Panel\\International\\sDate'),
    localeTimeSeparator = oSH.RegRead('HKCU\\Control Panel\\International\\sTime'),
    
    /* note: the iDate registry value can contain the following values:
    VALUE   MEANING
    0       mm/dd/yy
    1       dd/mm/yy
    2       yy/mm/dd
    */
    
    strDate = [
    
        [MM,DD,YYYY],
        [DD,MM,YYYY],
        [YYYY,MM,DD]
    
    ][localeDateFormat].join(localeDateSeparator),
    
    strTime = z(future.getHours()) + localeTimeSeparator + z(future.getMinutes());
    
    // feed calculated "stDate=date \n stTime=time" string back to batch
    WSH.Echo('stDate=' + strDate + '\nstTime=' + strTime);
    

    Even though that's much more complicated and much uglier than your original script, it should result in a nicer user experience. Your script will run from start to finish in a matter of seconds, rather than 30+ minutes.

    Update: The above script now gets the locale date format from the registry and formats the date appropriately.

    Here's another method, employing PowerShell to perform the date / time math. It's a little slower to execute, but significantly simpler.

    @echo off
    setlocal
    set "service=swiftalm"
    set "host1=Server_IP1"
    set "host2=Server_IP2"
    set "host3=Server_IP3"
    set "host4=Server_IP4"
    set /a timeout=600, loop = 1
    
    :loop
    echo;
    :: start first task immediately
    if %loop% equ 1 (
        echo ******Starting the Digite service on host%loop%******
        sc "\\%host1%" start %service%
        set /a loop += 1
        goto loop
    )
    :: else schedule for later remotely
    
    :: use PowerShell because cmd is horrible at date math.
    set /a i = timeout * loop - timeout
    for /f "tokens=1*" %%I in ('powershell -command "(Get-Date).AddSeconds(%i%) -f 'HH:mm'"') do (
        set "d=%%I"
        set "t=%%J"
    )
    
    echo ******Scheduling the Digite service to start on host%loop%******
    setlocal enabledelayedexpansion
    schtasks /create /tn "Start %service%" /tr "cmd /c sc start %service%" /sc once /sd %d% /st %t% /s !host%loop%! /z
    endlocal
    
    :: If more hosts in the list, increment loop counter and go back
    set /a loop += 1
    if defined host%loop% goto loop
    
    :: Runtime complete.
    echo All tasks complete.  Press any key to exit.
    >NUL pause
    goto :EOF
    

Ultimately, the only items of real importance that I feel strongly ought to be addressed are items 1 and 3. Everything else is just a matter of personal coding style and preference. Nevertheless, I hope this information is useful, or at least interesting.

\$\endgroup\$
  • \$\begingroup\$ Yes @rojo .. Things you explained in really very helpful and interesting. Thank you for the information you have provided. \$\endgroup\$ – Vaibhav Veralkar Jan 27 '15 at 5:31

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.