search

nezuo / lapis

A DataStore abstraction library for Roblox
https://nezuo.github.io/lapis/
MIT License
59 stars 10 forks source link

Make lock expire timer configurable #17

Open davidnurkkala opened 1 year ago

nezuo commented 1 year ago

Hi @davidnurkkala, I'm curious why you need to configure the lock expire duration. Are you running into situation where data isn't saving properly or do you think the default value isn't good? Do you need to configure it per Collection or can the setting be a global one instead?

I'm a little hesitant to make the value configurable because the user might choose a problematic value. For example, if they chose a value lower than the auto-save interval, session locking wouldn't work. We could error if they try to set it too low.

davidnurkkala commented 1 year ago

Hi,

So the issue I was running into was that occasionally servers might crash and thus fail to close documents. Now obviously I should fix the crash but it's a legacy project with a complicated 'base and I don't have any leads on it. I even have some Roblox staff looking into it.

Anyway, the documents may not get closed. My only option in this case is to kick players since I have no way to actually load their data. Now this isn't catastrophic as it's not terribly common for servers to crater in this way, but it feels pretty crappy to just kick players and tell them to come back in 30 minutes.

So I just wanted to lower it to 5, since that's not a super unreasonable period to wait in the odd case of server crashing. Figured I'd make it configurable. Did not consider the impact it would have on the rest of the API, though I expect 5 minutes is still a sufficient lock time?

nezuo commented 1 year ago

5 could be too low. The lock expiration timer should be greater than the time between saves, whether from auto-save or manual saves. The session lock will expire if you don't save in time. The auto-save interval is currently 5 minutes but it might take a little longer. You could also run into an issue where the server timestamps are slightly out of sync, that's why I chose a value of 30 minutes to be safe.

I'm still not sure about this change for those reasons, if you need this change as soon as possible, I would recommend using your fork instead if you aren't already.