0

On our macOs virtual machine, that we use for building, the time sometimes jumps out of no reason. As a workaround I have created this script named test.sh that consistently corrects the time:

#!/bin/bash -e

while true; do
    sudo ntpdate -u de.pool.ntp.org >> ntpdate.txt; sleep 30;
done

At the beginning of a build this gets started in the background:

./test.sh &

When the build is finished I'm killing it:

kill $(ps aux | grep test.sh | grep -v grep | awk '{print $2}')

Sometimes the call to update the time takes longer than 30 seconds. Then there are two open calls to the ntp pool and I'm getting a rate limit response. Therefore I want to limit the calls to ntp to only one at a time. How do I achieve this in my while true loop?

mles
  • 4,534
  • 10
  • 54
  • 94
  • 1
    You can try `kill %1` or `pkill -f test.sh -u $USER` instead of the complicated grep/grep/awk stuff. – John Zwinck May 18 '18 at 11:36
  • 2
    Your build process shouldn't have to worry about setting the time. Configure the VM to run ntpdate via `launchd` at the desired interval, and let the build processes assume the clock will be correct. – chepner May 18 '18 at 12:14
  • 1
    Your code *already does* proceed only when the previous command finished... unless, maybe, you have two separate builds happening at the same time, thus running two independent loops. – Charles Duffy May 18 '18 at 21:09
  • ...to handle the two-independent-builds case, see [Linux flock, how to “just” lock a file?](https://stackoverflow.com/questions/24388009/linux-flock-how-to-just-lock-a-file) – Charles Duffy May 18 '18 at 21:11
  • This should help... https://stackoverflow.com/a/37303133/2836621 – Mark Setchell May 18 '18 at 21:36

3 Answers3

1

A simple way to implement mutual exclusion in Bash is to use a "lockfile." You check if the file exists, and if it does, you do not do the NTP query. If the file does not exist, you create it. A useful enhancement in case of a crash will be to check if the file time is more than a few minutes old, in which case it can be deleted.

John Zwinck
  • 239,568
  • 38
  • 324
  • 436
  • There are a lot of perils to this -- a naive implementation can have two concurrent instances both see that there's no file, and independently proceed at the same time to the line that does the creation. Locking should really be done with `flock` to be safe, *or* with other tools that use `O_CREAT|O_EXCL` or directory semantics carefully (to be somewhat less safe, but somewhat more portable). – Charles Duffy May 18 '18 at 21:07
  • @CharlesDuffy: of course a naive lockfile implementation is not perfect, but I bet it is plenty good enough for OP's actual use case. But you're right, `flock(1)` is a good tool to use. – John Zwinck May 21 '18 at 03:33
1

Can you try to see if below works for your case.

#!/bin/bash

while true; do
  pid=0
  sudo ntpdate -u de.pool.ntp.org >> ntpdate.txt & pid=(${!})
  wait $pid
done
giddi
  • 103
  • 5
  • How is this different than not putting the process in the background in the first place, and consequently having the wait for its exit before flow control succeeds happen automatically/by default? – Charles Duffy May 18 '18 at 21:06
  • (making `pid` an array is also a little odd -- sure, `$pid` evaluates to the first element when it's given an array, but if you're only going to refer to that first element, why not keep the data type as a string?) – Charles Duffy May 18 '18 at 21:20
0

Thanks for all the suggestions. As Charles Duffy mentions my code already does proceed only when the previous command finished. I've tested it with:

#!/bin/bash -e

while true; do
    echo "start ntpdate"
    sudo ntpdate -u de.pool.ntp.org >> ntpdate.txt; 
    echo "going to sleep"
    sleep 30;
done

So apparently the sleep timer was too low to run into the rate limiting response. Maybe I will have to raise the sleep time.

As chepner suggested I created a LaunchAgent to handle this:

<?xml version="1.0" encoding="UTF-8"?>
<!DOCTYPE plist PUBLIC "-//Apple//DTD PLIST 1.0//EN" "http://www.apple.com/DTDs/PropertyList-1.0.dtd">
<plist version="1.0">
<dict>
    <key>Label</key>
    <string>workaroundAgainstJumpingTime</string>
    <key>ProgramArguments</key>
    <array>
        <string>sudo</string>
        <string>ntpdate</string>
        <string>-u</string>
        <string>de.pool.ntp.org</string>
    </array>
    <key>RunAtLoad</key>
    <true/>
    <key>StandardErrorPath</key>
    <string>/tmp/workaroundAgainstJumpingTime.stderr</string>
    <key>StandardOutPath</key>
    <string>/tmp/workaroundAgainstJumpingTime.stdout</string>
    <key>StartInterval</key>
    <integer>30</integer>
</dict>
</plist>

Charles Duffy, if you make an answer out of your comment "Your code already does proceed only when the previous command finished." I'll mark it as accepted answer.

mles
  • 4,534
  • 10
  • 54
  • 94