Julien Coloos wrote:
Hi,
As discussed on IRC last week, I worked on implementing MESSAGE quota
resource in cyrus (see RFC 2087; STORAGE resource already being handled).
I created a branch based on Greg's 'annotate' one on github, since his
work on annotation storage management made mine a lot easier.
Details on the changes I made:
cyrus-imapd:
- added MESSAGE quota resource management
-> updated cunit test
-> added 'quotawarnmsg' option which behaviour is similar to
'quotawarnkb'; warning message shown when selecting folder in IMAP
tells which resource limit was triggered
-> added 'autocreatequotamsg' option, to set MESSAGE limit
(unlimited by default) when user auto-creates its mailbox
- xfer transfers all non-unlimited resources
- added helper function to compute annotation storage usage for a
given mailbox
- changed the way quota entries are read/written
-> resources presence in fetched entry is remembered
-> when writing entry, only present resources are written
-> when setting resources limits, entries which were not present
upon fetching are now marked as present and their current usage is
computed
- quota utility lists and computes (-f) all resources associated to
mailbox
The branch 'quotamessage/gnb/annotate' is available here:
git://github.com/worldline-messaging/cyrus-imapd.git. It is based on
Greg's 'annotate' branch on github.
cassandane:
- added tests for MESSAGE resource
- modified current tests to play a bit with subfolders and setting
quota after adding messages/annotations
- special test to check new resource usage is computed when
necessary for pre-existing mailboxes (which only have usage for
STORAGE resource)
The branch 'quotamessage/gnb/master' is available here:
git://github.com/worldline-messaging/cassandane.git. It is based on
Greg's 'master' branch on github.
That all sounds promising and I look forward to reading it.
Things that may be worth noting:
- DUMP/UNDUMP currently does nothing special about MESSAGE or
X-ANNOTATION-STORAGE quota resources
-> should it be transferred ?
Yes. I've been lazy updating the dump/undump code.
-> without breaking backward compatibility, limits could only be
transferred through a 'fake' file entry, as for annotations
- quota usage is currently stored in a uquota_t variable, and delta
is computed as quota_t; so theorically there could be overflow issues
if quota usage to add/substract cannot be held in a quota_t; in
practice it should be unlikely since that would mean a usage of over
2^63-1
That seems...unlikely.
Why the change concerning quota entries read/write ?
The thing is that I wanted to make version upgrading as painless as
possible, both for users and in the code.
With previous code, when quota entry exists and is read, missing
resources are initialised with default values (0 usage, unlimited).
Thus only usage delta is tracked, and actual usage computing would
only happen if quota entry was missing: this is not nice when
upgrading, since lots of mailboxes are likely to have an entry with
only the STORAGE resource present.
So actual usage has to be computed at some point for newly handled
resources. The idea here is to compute it when setting the resource
for the first time. To do that it was necessary to know when the
resource was not previously present and keep it that way until its
limit is finally set for the first time.
This scheme has another advantage: for platforms where only STORAGE
quota is used, quota entries size remains as it is now. Only people
using those new quota resources will have their quota entries grow to
store the new data.
Comments are welcomed :)
I'll look at the code before commenting.
I think that there are two things that may also be done concerning
quota entries:
- always recompute resource usage when changing its limit from
unlimited to bounded
-> currently it is only done once when setting the usage limit for
the first time
-> that way, it may not be necessary to track resource presence
when reading/writing quota entries
-> but maybe it could be too time consuming in some cases, since
it seems to be possible to associate a quota resource to a whole
domain (recomputing usage for all mailboxes would then take some time)
- do not write resource in quota entry when its usage is unlimited
-> except for the STORAGE resource, for backward compatibility
-> would also help keeping quota entries size to the bare minimum
What do you think ?
This one's tough, I wasn't sure what to do. However, I'm happy to leave
it to the administrator to have to manually run quota -f (maybe twice!)
if they set a quota on a resource that is already being used. I'm
unconvinced that automatically doing the equivalent of -f as a side
effect of setting the first limit is necessary or wise. Perhaps we
should a) document it clearly and b) detect the situation and put an
obvious message saying something like "you may need to run quota -f ..."
where the human making the change will see it. Also, such a message
might be useful when usage underflow is detected.
PS: should I open a new bugzilla ticket for that ?
PPS:
cunit: on my computer cunit tests succeed except the 'foreach' one in
the 'quota' suite which timeouts (it seems messing with 4096
quotalegacy is too much for my computer).
Yeah, I hit a similar problem myself the other day, and solved it by
reducing the scale of the test so it completed in < 10 sec.
Alternatively you could bump up TEST_TIMEOUT_MS in cunit/unit.c.
cassandane: a few quota tests I added fail due to some of the issues I
reported (#5327 and #5329). And poor lemmings (well thought out name
in this context :)) are dying hopelessly and endlessly in the
Cassandane::Cyrus::Master test, preventing it to complete. Don't know
if it is normal.
Sorry, that's expected. The Master tests are exploring the master
process' resilience to certain misbehaviours in the services it starts,
and many of them will fail with the current master code. The fixes for
master and some cleanups are in
https://github.com/gnb/cyrus-imapd/commits/iris405 and will get merged
into master at some time in the future when I get around to it. In the
meantime I use the '!Master' argument to testrunner.pl to skip those tests.
--
Greg.
