-
Notifications
You must be signed in to change notification settings - Fork 179
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
yg-privacyenhanced.py error: jmclient.storage.StorageError: Read-only storage cannot be saved. #673
Comments
I have now done git reset --hard, fetch and rebase origin, updated to latest commit and restarted YG. Will monitor for more errors. |
So the question is why do you have a read-only storage. As far as I know, read-only is only a mode in which a wallet can be opened, and this is only currently used by Note that this is not related to file permissions; it's a feature of the I've never heard of someone trying to run yield generator scripts and having the wallet be read only, and I can't find it in the code. |
Thanks for your reply. I've completed many transactions on YG so far and this is first time this happened. Now, after update and restart of joinmarket, YG continue to operate normally and I had few transactions since yesterday, please see parts of my log:
It's running fine, storage is read write certainly, I haven't restarted my server, other programs are running fine. |
Interesting case. I'll have to squint at the code very hard because I can't figure out what might cause this. I mean clearly it sounds like a bug getting triggered in a rare edge case, but even if so, it's an unpleasant bug. |
Thanks for your interest in this. My system is nothing unusual, really. OS: Debian GNU/Linux 10 (buster) x86_64
|
My JoinMarket is as follows: cloned github repo to default folder. I start it each time by:
Every now and then, I do git reset --hard, git fetch, git rebase origin. python3 --version outputs Python 3.7.3. JoinMarket definitely uses it, as forcing it to python2 crashes, so definitely I am using python3. |
Just noticed similar issue on my test RaspiBolt setup. It had worked for some time normally before this, so there definitely isn't any configuration / permission issues. All possible involved partitions are rw.
|
Closing issue was accident, randomly hit some keyboard shortcut. |
If as you said on IRC it's still running, I'll be back in a few hours from now and have a think :) |
is there some weird sub-case where As I may have mentioned before, it is almost certainly this line that is causing the offence: ... the obvious question being, why would The above is just a summary of thoughts I already had in the previous case of this bug. I'll have another think shortly. |
Having looked at the code, I think the problem is here:
which is called if connection to bitcoind is lost here:
After calling |
Oh nice job @undeath thanks :) |
When I reported this issue at the time, bitcoind was running just fine on local machine. But I've watched recently Adam Gibson' presentation on Youtube (Bitcoin Meetup Munich, I watched 3.5hrs whole of it :) that latency (or quality of connection) to bitcoind is important and when someone is using remote bitcoind, it can cause issues and connectivity problems. Maybe my machine was I/O busy at the time (ElectrumX is eating a lot of I/O, and I do run other programs there) and there was not enough time for bitcoind to respond to JoinMarket? And it killed connection with bitcoin and then subsequently failed on read-only storage thing? Maybe there could be added waiting time for bitcoind to reply, some 60s grace timeout or something. |
Sorry about that 😆
This kind of thing seems to be fairly common, i.e. failures of RPC calls to bitcoind, especially if you're calling remotely, and yes probably having various system resource limitations/slowness may play a part too. We've had regular reports over the years of RPC calls failing, and there is a lot of exception handling code nowadays to ensure that the yg doesn't just fall over because of a temporary connection failure. After @undeath 's diagnosis above, it should be pretty clear for me to figure out how to fix this specific bug (I will start right now), albeit problems of this type are unlikely to permanently disappear. |
So I wonder if I can canvas opinion on this: This kind of problem comes out of a fairly deep uncertainty on what to do in cases where there are failures in JM "talking to Bitcoin Core". But there is a counterpoint: a big part of what yg does is to try to stay running for days, weeks, months at a time. It's hard if even one temporary failed network call crashes the program. To illustrate the point, see: joinmarket-clientserver/jmclient/jmclient/wallet_service.py Lines 53 to 60 in 6545f36
Notice here we panic. I want to tend towards the first of the above two points (I hope the argument is convincing). As a result I'm really not sure why the same reasoning isn't applied everywhere that joinmarket-clientserver/jmclient/jmclient/wallet_service.py Lines 280 to 281 in 6545f36
... which is called on every tick of the transaction monitor loop. Currently this is where the bug will occur, because you are in a normal running state, the rpc call for blockheight fails, Honestly it's kind of tricky. |
I enjoyed it and learned a lot. 🥇 👍 It was you now I realized :) Thanks. Perhaps easy solution can be implemented for now (crash hard with suitable error message so we know what happened, do not let it hang in zombie mode), and later implement solution to wait gracefully for Bitcoin Core to respond? At least 10 seconds wait would solve this I guess. By the way I have 2x 2TB drives in mdadm RAID1 mode for /home, they are quick and in very good condition, so it's not like it's dying, obsolete and faulty hardware generate this errors. System is on mdadm RAID1 too (2xSSD) and all is powered by 16-threaded Ryzen 7. Bitcoind and electrumx thrashing HDDs and pounding on CPU all the time and they never moan. |
So there's been a little back and forth on IRC, but I want to summarize it here for future reference (although you can find today's log at gnusha.org/joinmarket/): -> Does the WalletService really need to call -> After some time I remembered where that call to Long running programs like the JoinmarketQt GUI will want to open, close multiple wallets, including the same wallet, and since the wallet is locked until it is closed, it means we need to close the wallet and shutdown the monitor service when opening a new wallet in Qt. See #595 . -> Whilst this is true, it doesn't address the point raised in this Issue; what do we do when an RPC call fails such that our blockchain access is apparently non-functional? We cannot leave it as-is, it seems, since stopping the service and closing the wallet means the Maker is no longer monitoring the blockchain, and cannot access the wallet (as he needs to of course, when negotiating a transaction, starting in -> But simply not stopping the service and not closing the wallet is in my opinion not a solution, largely because of the concerns outlined above: we don't know that we have access to up to date bitcoin blockchain information, so it is not really safe to continue. There are therefore two approaches, as I believe I mentioned elsewhere a couple of months back: either "shut down everything" or find a way to pause the entire running of the maker until we know RPC access is working correctly. The latter is the "right" solution but is going to take significant work. Since I, @undeath and @curious0101 above have now said largely the same thing, I think the only sensible thing is to write a short patch that gracefully quits the application when we fail to read the blockheight in |
Today I've noticed error in YG terminal output. After this error, YG continued to operate and log continued to populate with debug and info messages.
My storage is fine, there is no hardware problems, everything works perfectly on my server and certainly there is read-write rights to my /home partition where joinmarket folder reside. I'm using joinmarket from git, latest commit is:
commit 28ce6ab4596df7c34b96e0d1ba777861c72cb70b (HEAD -> master, origin/master, origin/HEAD) Merge: 8b980c1 8de8381 Author: Adam Gibson <ekaggata@gmail.com> Date: Mon Aug 10 15:37:32 2020 +0100
The text was updated successfully, but these errors were encountered: