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

Make lock expire timer configurable #17

Open
wants to merge 5 commits into
base: master
Choose a base branch
from

Conversation

davidnurkkala
Copy link

No description provided.

@nezuo
Copy link
Owner

nezuo commented Aug 28, 2023

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
Copy link
Author

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
Copy link
Owner

nezuo commented Aug 29, 2023

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants