Improve /quota-update endpoint

[sergejostir]

I would like to suggest two improvements for this endpoint:

1. Support "quota_size" and "quota_files" fields. This endpoint doesn't support updating these two fields and forces us to use "update user" endpoint, but then we have to force disconnect users if we want to activate these changes.

2. If a field is omitted do not set it to 0. I don't know why this is like this currently, since if a user intends to set it to 0, they can do that explicitly. This cahnge would allow us to easily change only the fields that we want.

[drakkan]

I would like to suggest two improvements for this endpoint:

1. Support "quota_size" and "quota_files" fields. This endpoint doesn't support updating these two fields and forces us to use "update user" endpoint, but then we have to force disconnect users if we want to activate these changes.

these values are loaded only when the user logins, if you change them they will be applyed the next time it logins

2. If a field is omitted do not set it to 0. I don't know why this is like this currently, since if a user intends to set it to 0, they can do that explicitly. This cahnge would allow us to easily change only the fields that we want.

This happen because Go assume 0 for int64 if you don't pass it. I can fix this by making the receiver a pointer and checking if it is not nil

[drakkan]

I don't know, I have to think about it.

The quota-update endpoint actually allows to externally update the quota usage, you have no other way to update used_quota_files and used_quota_size, so this is a specific use case.

To allow this feature we have to add locking everywhere or read these values from the data provider each time, but this isn't the main point.

Other SFTPGo users can then ask to update other fields without disconnecting the user and so on, we'll end up updating the whole user. This was already asked in the past, see #163, and it is very complex to handle, imagine what happen if you change home dir or fs provider for a logged in user. Updating a connected user will also break the usage you described in #387.

Maybe we could do something different such as a pre-upload hook that allows to externally check some conditions and deny an upload based on the response received from this hook.

Can you please better explain your SFTPGo usage for quota checking? If track_quota is !=0 in your configuration file then SFTPGo will update quota usage, if you also call quota-update the stored quota will be wrong. Thank you

[sergejostir]

I agree that this could let to a question, why is then not everything editable without reconnecting.

To allow this feature we have to add locking everywhere or read these values from the data provider each time, but this isn't the main point.

What do you mean?

Can you please better explain your SFTPGo usage for quota checking? If track_quota is !=0 in your configuration file then SFTPGo will update quota usage, if you also call quota-update the stored quota will be wrong. Thank you

The folders that are accessible via sftpgo are also used by different processes to write their files to. So I check disk usage every 30s to make sure that they are not over their limits (if they are they are killed). /quota-update is used to inform sftpgo of the current usage (because it can't know about creations/deletions from external processes). Sftpgo checking mechanism is used so sftp users are staying within quota limits. Quota limit can also change sometimes, so that means that sftp user has to reconnect, but that's rare, so I guess this change is not that important, if it takes too much effort.

[drakkan]

I agree that this could let to a question, why is then not everything editable without reconnecting.

To allow this feature we have to add locking everywhere or read these values from the data provider each time, but this isn't the main point.

What do you mean?

currently when the user login a session is created and it stores the user struct. The user is never updated, if we start to update the user we need to add a mutex

Can you please better explain your SFTPGo usage for quota checking? If track_quota is !=0 in your configuration file then SFTPGo will update quota usage, if you also call quota-update the stored quota will be wrong. Thank you

The folders that are accessible via sftpgo are also used by different processes to write their files to. So I check disk usage every 30s to make sure that they are not over their limits (if they are they are killed). /quota-update is used to inform sftpgo of the current usage (because it can't know about creations/deletions from external processes). Sftpgo checking mechanism is used so sftp users are staying within quota limits. Quota limit can also change sometimes, so that means that sftp user has to reconnect, but that's rare, so I guess this change is not that important, if it takes too much effort.

Ok, it is clear now. This isn't an easy change. I could consider it at the beginning of the next development cycle (after 2.1.0)

[sergejostir]

currently when the user login a session is created and it stores the user struct. The user is never updated, if we start to update the user we need to add a mutex

I see, but how is updating used_quota_size and used_quota_files handled then? I though all these values are simply stored in session, and when quota check is needed you simply compare if (used_quota_* + additional size/count) > quota_*, so changing them anytime wouldn't be a problem since they do not have any "long running" effect.

Ok, it is clear now. This isn't an easy change. I could consider it at the beginning of the next development cycle (after 2.1.0)

If this is so different than changing used_quota_*, then I guess just leave it and better devote time for something more important.

[drakkan]

currently when the user login a session is created and it stores the user struct. The user is never updated, if we start to update the user we need to add a mutex

I see, but how is updating used_quota_size and used_quota_files handled then? I though all these values are simply stored in session, and when quota check is needed you simply compare if (used_quota_* + additional size/count) > quota_*, so changing them anytime wouldn't be a problem since they do not have any "long running" effect.

these values are not part of the user session, you cannot update them when you modify a user. If we store the used quotas within the session and you have the same user logged in multiple times the used quota values will be wrong.

For this reason each time we need to update or check the quota usage we do a data provider query. This behaviour leads to performance issues if you have quota restrictions and you upload a lot of small files, for this reason I added delayed quota updates

c844fc7

Using delayed quota updates the used quotas will be stored in memory for a configurable time, for example 60 seconds, and persisted to the data provider each 60 seconds. If in 60 seconds you upload 1000 files this will result in 1 one data provider update instead of 1000 updates.

We could read several or all the fields from the data provider instead of storing them within the session and so we always use the updated values but this way we'll do a lot of queries and will have poor performances and the change to parameters like the user home dir will be hard to manage anyway

Ok, it is clear now. This isn't an easy change. I could consider it at the beginning of the next development cycle (after 2.1.0)

If this is so different than changing used_quota_*, then I guess just leave it and better devote time for something more important.

[drakkan]

I would like to suggest two improvements for this endpoint:

1. Support "quota_size" and "quota_files" fields. This endpoint doesn't support updating these two fields and forces us to use "update user" endpoint, but then we have to force disconnect users if we want to activate these changes.
2. If a field is omitted do not set it to 0. I don't know why this is like this currently, since if a user intends to set it to 0, they can do that explicitly. This cahnge would allow us to easily change only the fields that we want.

I'm too tired to test this now, I'll check it tomorrow. Based on the code here, the omitted field is setted to 0 and so it should not change the actual value, it should create an sql update like this one SET used_quota_size = used_quota_size + 0 and so should not change the actual value. The used quota size will be set to 0 only if you reset the quota instead of incrementing it, but passing both the values should be expected in the reset case

[daciodf]

Hello everyone,

Can someone help me how to set a default quota for all users who log into sftpgo. I'm using helm chart in kubernetes.