Save options to disk whenever they change

fronobulax

Developer
Staff member
It seems like checking that option has been presented as the solution to a couple of recent issues. Why is it a user option and not just a system default? In other words why does KoLmafia not save unconditionally on changes?

I have some vague memories that the original implementation relied on some Java file I/O to be reliable across operating systems and it turned out that was not the case. Even under normal shutdown conditions, sometimes cached data did not get dumped to disk. So the flag was introduced to work around that.

Why should it be an option today? If people have really, really slow disks, I can imagine disk I/O having a noticeable impact. If people are using SSDs I can imagine they want to minimize writes to preserve disk life but if that is really something KoLmafia should support then we should probably revisit sessions logs as well. But those feel like niche cases to me.

So should changes be written to disk immediately and unconditionally?

I'm thinking yes and then wondering if there are some alternatives to Java Files to be investigated. How wrong am I? ;-)
 

lostcalpolydude

Developer
Staff member
Having that setting enabled means that if mafia is updating a setting when your OS crashes, you lose everything (I've seen this happen as a 20kB file filled with null bytes). Having it disabled in that case means that you just lose stuff from the current session.
 

roippi

Developer
From a developer's perspective, it's kind of a hacky implementation. Rewriting the whole prefs file every time a single pref changes is kind of a drag, since we update prefs -a lot- and the file itself can range into the hundreds of KB. Java is fast enough that you probably won't notice it too much, but I wouldn't be surprised if it adds some nonzero number of seconds to an average user session when turned on.

Also as I previously noted somewhere, we don't dispatch a separate thread to do the writing. We /could/, and basically negate any performance downside by doing so, but we don't. Also this implies that it is currently possible that two threads change a pref at the same time, causing simultaneous writes (maybe, depending on the OS) and thus file corruption. This could be addressed trivially by adding some mutex condition in there.

Aside from the performance issue which is easily addressed, the added i/o strain on HDDs might be a real issue. I haven't dived in to the nitty-gritty to see how much of the i/o is just buffered out by the OS/HDD firmware and how much is real, so I can't say how big of an issue that really is. Though again if we really cared about it, we could address this by implementing some caching and only firing off a dump-to-pref file every 10 seconds or so. I think I'd want both of these features implemented if we removed the saveSettingsOnSet pref and just made it mandatory.
 
Last edited:

roippi

Developer
Having that setting enabled means that if mafia is updating a setting when your OS crashes, you lose everything (I've seen this happen as a 20kB file filled with null bytes). Having it disabled in that case means that you just lose stuff from the current session.

We should probably be writing to a tempfile and then moving that over the old pref file so this doesn't happen. I think that that transaction is atomic.
 

fronobulax

Developer
Staff member
Having that setting enabled means that if mafia is updating a setting when your OS crashes, you lose everything (I've seen this happen as a 20kB file filled with null bytes). Having it disabled in that case means that you just lose stuff from the current session.

To the extent that this is a probable failure mode, I am concerned that the default is true. I note that the default was added or changed to true way back in r6272 by veracity so that a user who claims they never set the value to true is either misinformed or has a settings file that has remained intact enough that the former default value of false was preserved.

Regardless of philosophy, it seems clear that eliminating the preference is not the way to go because this way users get to pick which failure mode they fear most.

The right solution requires more thought and work. Thanks.
 

Fluxxdog

Active member
I have a question, but I'd like to explain my thinking behind it.

When I look at the settings, I see several collections. GLOBAL_aliases, _moods, and _prefs, and then xxx_moods and _prefs. Essentially 5 different files. Each xxx_prefs file has 4 different types of preferences.
*Ones that are designed to change rarely, such as betweenBattleScript.
*Then there are those that change once per ascension, like controlRoomUnlock.
*Then there are the daily ones, like all the _yyyyyyy and some of the older ones like cameraUsed.
*Finally, you have choice adventures, which can be a cross between 1/ascension and daily.

Would there be any benefit to splitting them up?
 

Fluxxdog

Active member
Do you consider a coding nightmare to be a benefit?

Other than for practice, no ^^ But I mean as far as the end goal of resisting sudden failure as mentioned above. If a person has his breakfast set, why write his breakfast preferences each time he summons something from a libram? Would there be any advantage to this as opposed to the current system, or would the overhead outweigh the benefits.

Yes, I realize it would be quite the hassle to code, but I'm asking as more of a thought experiment rather than "HEY! DO THIS BLINDLY!" I'm not as experienced in Java as the lot of you, so I'd like to know why decisions are made.
 

Veracity

Developer
Staff member
The problem is, it doesn't solve the "problem" - which seems to be "if my OS has a hard crash while writing out my preferences file, it gets corrupted". Note that that can happen whether you write the preferences many times during a session, or just once, and whether the preferences file is big (has everything) or there are many smaller files.

Categorizing preferences and having a file for each category would would be a huge job - both to make the call over which preference goes where, and to actually change the code to do it - and it wouldn't solve anything. It would just mean that if the OS crashes, only some of your settings get corrupted.

I think the only way to make the saving resilient from crashing in the middle would be to write a temp file and then move it to the real file, as discussed. I wonder what happens when the OS crashes in the middle of the File System modification?
 

Fluxxdog

Active member
So a redundancy setup would be more likely to get back if there is a catastrophic failure. I see where you're coming from now. Thanks!
 

fronobulax

Developer
Staff member
At some point we have to deal with the possibility that not every failure mode should be handled at the programming level or even worried about at all. I have not had an OS crash on any machine in several months and I don't think I ever had a crash while KoLmafia was the running program. I'm running Windows and I've been told reliability of this sort is much better for other operating systems :) My personal experiences with preference corruption all seem to have involved multiple instances of mafia running or a non-mafia tool doing something with the files, perhaps while they were "open" in mafia. As these are identified code has been changed.

I am hard pressed to identify any software I use or encounter that is expected to survive or recover from a hard OS crash while doing disk IO. The obvious exceptions are databases and/or transaction processing systems. When properly configured these can treat disk I/O as an atomic event that either succeeds or fails. The programmer who cares will detect the failures and take appropriate action. The database start up and shutdown processes make an effort to detect that there are write failures or incomplete transactions and rollback or recover. But even then it is theoretically possible to lose data. The next layer is things like RAID arrays and redundant mirrored servers.

Back to the realm of KoLmafia, if it were important enough I would consider one of the following: First there are embedded or standalone SQL databases that are highly unlikely to lose data if they are set up and managed towards that goal and the programmer uses them with care. So making all the preferences entries in a SQL database and let the database deal with disk IO is one option, although overkill might be the operative word here. The second option would be to roll our own using Java best practices. That means we have an internal cache and preference reads and writes check the cache first. There is a monitor thread that writes the cache to disk periodically where the frequency is a trade off between safety and performance. Disk writes are to a temp file and something has the job of backing up the original and replacing it with the temp. Finally KoLmafia start up and shut down need to detect that there may have been a problem and attempt to recover. And all of this has to work without file related collisions at the OS level in case there are multiple instances of mafia running.

Being lazy and confident that the high probability of occurrence bugs have been squashed if I wanted to move forward on this I would review the existing code to make sure it is as failsafe as the implementation will support and then consider a Feature Request whereby the most critical items had redundant storage. KoLmafia reading and writing Demon Names to the KoL Quest Log would be an example of this (but even that opens up its own can of worms). But having raised the question, I'm not sure the benefits of a change are worth the cost.
 

lostcalpolydude

Developer
Staff member
KoLmafia reading and writing Demon Names to the KoL Quest Log would be an example of this (but even that opens up its own can of worms).

If something like this was done, the CAB would be a better place than the quest log. Xenophobe documented that capability, and kolproxy uses it. I've seen a greasemonkey script regularly wipe the CAB doing something wrong, and I know of one person that doesn't use kolproxy because their network setup means that it wipes their CAB, so it is a complicated thing to get right.
 

Veracity

Developer
Staff member
Yeah, I have been interested in using the CAB ever since Xeno implemented it, but two things have stopped me from trying it:

- If we screw up, we wipe the CAB, losing our preferences, the user's CAB buttons, and every other script's user data stored there.
- If another script screws up, the above happens.

We have also considered using the Quest log for this kind of thing, since before the CAB was implemented. But, as frono says, that is its own can of worms.

In the end, I think our own settings file is the best - and put it on Dropbox, if you want to share it across machines.
 

Darzil

Developer
First there are embedded or standalone SQL databases that are highly unlikely to lose data if they are set up and managed towards that goal and the programmer uses them with care. So making all the preferences entries in a SQL database and let the database deal with disk IO is one option, although overkill might be the operative word here
I believe Microsoft was planning to do that for it's server O/S at least, so the file system was actually a SQL database. Not sure what ever came of that, it's was years back.
 
Top