jelmer / xandikos

A CalDAV/CardDAV server backed by Git
https://www.xandikos.org/
GNU General Public License v3.0
423 stars 42 forks source link

RFC - make indexing threshold configurable #223

Closed tobixen closed 2 years ago

tobixen commented 2 years ago

There is an index cache in Xandikos which may cause slightly different code paths to be used on the first six executions and the seventh and later execution of certain parts of the code. The number 5 is currently hard coded in xandikos.store.index.INDEXING_THRESHOLD (and with counting starting at 0, this causes the 7th execution to be the first one using the index - this may be a bit confusing).

I think it would be good to make this configurable, because:

1) Generally, things like this should not be hard coded - different usage patterns, resource constraints and the like may cause the number 5 to be optimal at some installations of xandikos, and non-optimal at others.

2) For testing purposes, it may be nice to set it to 0 (or even -1), to ensure the indexing code path is exercised.

3) In other settings it may be nice to set it to a very high number while waiting for a bugfix to be released - or to verify that a bug is related to the indexing.

I see three ways of doing it:

1) Receive command line options in the execution script and make sure it passes through to the IndexManager constructor. To me this looks like the most proper solution, but it is also quite some code and "noise" for an option that very few will ever have the need to touch.

2) Allow the "constant" to be changed prior to instantiating the index manager object. This configuration would then be unavailable for most people, but possible to tweak i.e. when running unit tests. For this to work, I think it's just to do something like this:

--- a/xandikos/store/index.py
+++ b/xandikos/store/index.py
@@ -82,7 +82,9 @@ class MemoryIndex(Index):

 class IndexManager(object):
-    def __init__(self, index, threshold=INDEXING_THRESHOLD):
+    def __init__(self, index, threshold=None):
+        if threshold is None:
+            threshold = INDEXING_THRESHOLD
         self.index = index
         self.desired = collections.defaultdict(lambda: 0)
         self.indexing_threshold = threshold

3) Let it be configurable through environment (may be combined with method 2 above). I believe something like this would work:

--- a/xandikos/store/index.py
+++ b/xandikos/store/index.py
@@ -23,8 +23,11 @@
 import collections
 import logging

-
-INDEXING_THRESHOLD = 5
+try:
+    import os
+    INDEXING_THRESHOLD = os.environ.get('XANDIKOS_INDEXING_THRESHOLD', 5)
+except:
+    INDEXING_THRESHOLD = 5

 class Index(object):

(everything under the os module may be os-dependent - there is no guarantee that os.environ exists on all operating systems - hence the try-except block)

I can probably write up a pull request with tests on this one, though no guarantees on when/if I will have the time for it.

jelmer commented 2 years ago

This is already implemented - see #222

tobixen commented 2 years ago

Damn, I'm slow :-)