Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Database entry(ies) regularly wiped #391

Open
kurkale6ka opened this issue Nov 16, 2015 · 38 comments
Open

Database entry(ies) regularly wiped #391

kurkale6ka opened this issue Nov 16, 2015 · 38 comments
Assignees

Comments

@kurkale6ka
Copy link

Hi,

This is my issue: I keep using j pu which brings me to a specific directory. This works for a certain amount of time, sometimes weeks, sometimes days, then one day it would report . and not be able to jump to my dir. There is nothing in the help that suggests cleaning of the db or anything, I can only guess what's going on... but I have no idea

@sbuljac
Copy link

sbuljac commented Nov 23, 2015

Hi,

Same issue here except I think it only happen after the reboot.

@trou
Copy link

trou commented Jan 4, 2016

Probably fixed by #383

@trou
Copy link

trou commented Apr 1, 2016

Apparently not fixed. This is really annoying. Any idea ?

@wting
Copy link
Owner

wting commented Apr 1, 2016

Sorry for the late response but I have a rough idea of what's happening and how to fix it.

On every directory change autojump locks the data file and updates an entry, stored in a "database" like format. There's a race condition (exacerbated by shell scripts / commands that traverse directories like find) where the lock fails and the db gets overwritten.

The proper way to fix is either:

  1. Fix the file locking to be race condition safe.
  2. Switch from a database like format to an append only log (like .bash_history, .zsh_history, etc), and only calculate the weights when someone invokes autojump to switch directories (aka j).

@wting wting added the bug label Apr 1, 2016
@trou
Copy link

trou commented Apr 4, 2016

Hmm I just looked at the code and I don't see any actual locking going on.
Moreover the backup is done right after the move from the temp file to the real DB, so if the move failed (there's no error checking), we're backuping an empty file.

I guess improving this should'nt be that hard.

@azat
Copy link

azat commented Jun 7, 2016

Maybe #383 didn't guard all places with flock, but there is more simple approach, but just wrapping the whole autojump inside flock, like:
alias autojump='flock /tmp/autojump.lock autojump'

@trou can you test this?

UPD: fix syntax issue

@trou
Copy link

trou commented Jun 7, 2016

I just added it to my bashrc, we'll see.

@trou
Copy link

trou commented Jul 18, 2016

It doesn't seem to help :(

@azat
Copy link

azat commented Jul 19, 2016 via email

@trou
Copy link

trou commented Jul 19, 2016

I don't know but the issue should be fixed in the code anyway.

@nvgoldin
Copy link

I been using autojump for around a year now, and suddenly - it looks like it cleaned out the entire history. Looking at ~/.local/share/autojump, I see a new file was created. I saw #208, which points here. I'm using:
autojump-22.3.2-1.fc24.noarch

Any other debugging info I can provide?

@Micha-Btz
Copy link

I also have this regularly is there a way to debug this?

@r-barnes
Copy link

@wting: You mentioned that one way to resolve the problem might be to switch to an "append only log" and calculate weights when autojump is run.

I suppose you are avoiding this solution because the log might become prohibitively long over time. (And, also, because it would be nice if the existing system worked in the way we think it should.)

But perhaps a hybrid solution is possible? Append entries to a log, but add an autojump command that converts them to the database format. When autojump is run, consult both the log and the database to calculate the actual weights.

The foremost downside to this is that it requires the user to collapse the database manually. A work around would be to do a random test each time autojump is run to determine if the db should be collapsed. This would, at least, decrease the odds of a race condition.

@r-barnes
Copy link

@azat: I'm going to try your solution.

@PorcelainMouse
Copy link

Since I implemented a patch proposed in #482, I have not experienced this bug.

I'm interested in hearing @r-barnes results, too, though. If successful, is there a reason this "early locking" could not be used in autojump?

@PorcelainMouse
Copy link

Argh! Got an update that overwrote the patch from #482. It happened on the next reboot. I don't understand how other people aren't experiencing this. It happens to me constantly.

@r-barnes
Copy link

I did not have 100% success with #482. The database still seems to clear or, perhaps, is limited in size. It doesn't seem to grow much beyond 23 entries for me.

@PorcelainMouse
Copy link

I did have 100% success with #482. April 21 until July 25. I have 279 entries currently. This suggests our situations are different, which means there are multiple triggers for this bug. Whatever I'm doing that causes it, is always related to the different filesystems that autojump uses to managed the db, like @Frefreak suggested. And, just as I suggested, there could be multiple code paths that lead to the same bug, some of which, I just never use but you do.

@lilydjwg
Copy link
Contributor

Ah, it happened again someday last week, after so long a time since last time.349 entries was removed, file size shrank from 93K to 77K.

@ericyue
Copy link

ericyue commented Mar 10, 2018

any updates?

@kurkale6ka
Copy link
Author

I've ended up developing my solution (zsh only). Way smaller, more useful and more reliable :)
https://github.com/kurkale6ka/zsh (the README on that link)

@PorcelainMouse
Copy link

Looks like here are several existing alternatives. I'm trying 'fasd' now https://github.com/clvv/fasd , which is just one.

@dfaure-kdab
Copy link

See rupa/z#198 -- similar bug report in another project that has a similar issue.
I can't find a working autojump-like solution :(

@lilydjwg
Copy link
Contributor

lilydjwg commented Sep 7, 2018

FYI, I haven't had this issue for more than one year after I changed the temporary file to the same directory as the data file. The patch is:

--- autojump_data.py    2018-09-07 15:28:30.488681864 +0800
+++ /usr/bin/autojump_data.py   2017-08-26 15:43:50.136781805 +0800
@@ -120,11 +120,12 @@
 
 def save(config, data):
     """Save data and create backup, creating a new data file if necessary."""
-    create_dir(os.path.dirname(config['data_path']))
+    data_dir = os.path.dirname(config['data_path'])
+    create_dir(data_dir)
 
     # atomically save by writing to temporary file and moving to destination
     try:
-        temp = NamedTemporaryFile(delete=False)
+        temp = NamedTemporaryFile(delete=False, dir=data_dir)
         # Windows cannot reuse the same open file name
         temp.close()
 

Moving files across devices is not atomic (it does a copy + delete operation) so they should be put in the same device to overwrite atomically.

@dfaure-kdab
Copy link

That makes a lot of sense to me. A move is only atomic if it's on the same partition...

wting added a commit that referenced this issue Sep 7, 2018
One idea for why the database is occasionally purged is due to the
temporary file being on a separate device than the database. This
patch implements @lilydjwg's suggestion here:

    #391 (comment)
@wting
Copy link
Owner

wting commented Sep 7, 2018

This is implemented in v22.5.2 here: bc4ea61

For now please install and test from source and report back if data is still being lost.

@lilydjwg
Copy link
Contributor

lilydjwg commented Sep 8, 2018

I find that actually I opened a pull request #495 with the change last year, but it went unnoticed.

@wting
Copy link
Owner

wting commented Sep 9, 2018

Oops, I reverted bc4ea61 and merged your changes into 8fffbad.

@Micha-Btz
Copy link

I'm not sure if the time is enough for testing, but I don't have problems found with database wiping. What do you think @wting. And if it is right for you, can you create a new release, so that distros can it picked up?

@minjang
Copy link

minjang commented Nov 30, 2018

I started to suffer from this wiping problem. After several hours, autojump.txt is wiped out. Anyway to debug it?

@kaihendry
Copy link

I recently noticed autojump failing to work as expected. Then I checked to find:

[hendry@t480s ~]$ wc -l ~/.local/share/autojump/autojump.txt
35 /home/hendry/.local/share/autojump/autojump.txt

Er... it appears to have been reset. Any ideas WHY?

@wting
Copy link
Owner

wting commented Apr 8, 2019

There was a race condition that occasionally wiped the database entry that was fixed in v22.5.3.

@minjang, @kaihendry: Can y'all run autojump -v and share the version number listed? If it's less than that version then please upgrade and/or install from source.

@kaihendry
Copy link

[hendry@t480s ~]$ autojump -v
autojump v22.5.1
[hendry@t480s ~]$ pacman -Qi autojump
Name            : autojump
Version         : 22.5.1-2
Description     : A faster way to navigate your filesystem from the command line
Architecture    : any
URL             : https://github.com/wting/autojump
Licenses        : GPL3
Groups          : None
Provides        : None
Depends On      : python
Optional Deps   : None
Required By     : None
Optional For    : None
Conflicts With  : None
Replaces        : None
Installed Size  : 121.00 KiB
Packager        : Felix Yan <felixonmars@archlinux.org>
Build Date      : Sat 10 Nov 2018 06:58:45 AM
Install Date    : Wed 14 Nov 2018 08:57:48 AM
Install Reason  : Explicitly installed
Install Script  : No
Validated By    : Signature

I'm an Archlinux user btw 🤣

@r-barnes
Copy link

r-barnes commented Apr 8, 2019

In 18.04.2 LTS, I have v22.5.1. I'm not sure if the update has made it to the Debian/Ubuntu repos, or if the version's been frozen. Installed update: we'll see how it goes! (Thanks so much.)

@wting
Copy link
Owner

wting commented Apr 8, 2019

I'm not sure who maintains the Debian package for autojump these days, but it's possibly that they only build off stable tags. While master branch has been on v22.5.3 for >6 months, I only tagged v22.5.3 tonight.

Your best bet to deal with this random data loss is to install from source following these instructions.

@sergiomb2
Copy link

Hi, Since I updated autojump to 22.5.3 I haven't seen this issue again, more than one year until now.
Seems it is fixed

@dfaure-kdab
Copy link

Same experience here. I think this issue can be closed.

@selurvedu
Copy link

+1, seems to be fixed.

cryptolover7 added a commit to cryptolover7/autojump that referenced this issue Jul 27, 2022
One idea for why the database is occasionally purged is due to the
temporary file being on a separate device than the database. This
patch implements @lilydjwg's suggestion here:

    wting/autojump#391 (comment)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.