Why not automatically remove stale lock directories?

[duasfl8r]

Whenever a sync is interrupted (e.g. manually with Ctrl-C), the lock directory is left behind, with a file containing the PID of the process that created it.

When it tries to sync again, the function acquire_lock tries to kill the process with this PID, and if it fails (i.e. the process does not exist), it exits with this error message:

There's stale lock directory at <LOCK_DIR>. Please remove it and try again.

Why not remove the lock directory directly in the script, instead of asking the user, if it is known to be stale?

Is there a case in which there is a stale directory but removing it could be harmful?

[ku1ik]

acquire_lock does not try to kill the process. It does kill -0 $PID which is like "ping" to the process, killining nothing. Exit status of this kill command tells if the process with this pid still runs or not.

[adityamukho]

Perhaps the following wrapper script (to run bitpocket on the slave), can be enhanced to handle Ctrl-C interrupts? Run it with the synced folder as the first argument.

#! /usr/bin/env bash

cd $1
PIDFILE="$1/.bitpocket/run.pid"

sleep $[ ( $RANDOM % 300 ) ]s #Reduce collisions with other slaves trying to sync simultaneously.

if [ -e "${PIDFILE}" ] && (ps -u $USER -f | grep "[ ]$(cat ${PIDFILE})[ ]"); then
echo "Already running."
exit 99
fi

rm -rf .bitpocket/tmp/lock #Previously spawned proc is now dead. There should be no lock at this point. This step corrects for an unclean shutdown.
/usr/bin/bitpocket cron &

echo $! > "${PIDFILE}"
chmod 644 "${PIDFILE}"

Ref: http://notesfromheck.wordpress.com/2012/09/17/syncing-with-bitpocket-a-flexible-open-source-alternative-to-dropbox

Edit: Actually this is already taken care of in the next run of the script.

[ku1ik]

@LucasTx thinking about why not to remove stale locks automatically... and I can't remember any good reason at the moment. I'll take a look at test cases and the script itself and will try to figure out if there's any danger in automatic removal.

[ku1ik]

Well, I remember now. The stale local lock signals that previous sync didn't finish with success. I am not sure if we can just run another sync without user first checking if the files/directories are looking fine after previous unsuccessful sync. Yeah, so the purpose was to block next sync and require user to "unblock" it by remove the lock manually.

Do you think the default behavior should be changed? Also, maybe we can just add "-f" flag to force sync with automatic lock removal.

[duasfl8r]

The "-f" flag option seems to be the best option, as it will keep the
original behavior intact and allow people like me to ignore these blocks
(as they happen quite a lot in my situation).

On Wed, Jan 9, 2013 at 6:18 PM, Marcin Kulik notifications@github.comwrote:

Well, I remember now. The stale local lock signals that previous sync
didn't finish with success. I am not sure if we can just run another sync
without user first checking if the files/directories are looking fine after
previous unsuccessful sync. Yeah, so the purpose was to block next sync and
require user to "unblock" it by remove the lock manually.

Do you think the default behavior should be changed? Also, maybe we can
just add "-f" flag to force sync with automatic lock removal.

—
Reply to this email directly or view it on GitHubhttps://github.com//issues/30#issuecomment-12064542.

[torfason]

The current version of bitpocket spits out a command that the user can run to remove the lock file, making everything very simple but also transparent. It does not remove the lock file on the server. To close this issue, it seems that there are two things needed:

1. Come up with a command to remove the lock file on the server, analogous to the one that already removes the client lock file, and spit it out if a server lock file is found.
2. (Optionally) add a -f option that runs those two commands automatically. Removing the lock file on the server is even more intrusive than removing the lock file on the client, though, so it is not obvious to me that the -f option is needed.

[greezybacon]

I think we should look to make this an automatic process or otherwise write something to stderr so that the crontab system will send me a mail when the sync breaks. Otherwise the sync may stop running for a number of days before I notice.

[greezybacon]

I've implemented the -f option in #74 and merged it to the develop branch. I still like the warning idea too