Solr
  1. Solr
  2. SOLR-571

LRUCache autowarmCount should support percentages

    Details

    • Type: Improvement Improvement
    • Status: Closed
    • Priority: Major Major
    • Resolution: Fixed
    • Affects Version/s: None
    • Fix Version/s: 4.0-ALPHA
    • Component/s: None
    • Labels:
      None

      Description

      a recent thread reminded me of an old idea that never got implemented...

      the autowarmCount for LRUCaches should support "percentages" which get evaluated relative the current size of the cache when warming happens.

      in this way, a Solr instance might be configured with autowarmCount="50%" to autowarm the top half of the queries ... a master machine might be configured this way to give some autowarming if it is inadvertently being queried, but things won't be maintained in the caches in perpetuity ... each subsequent cache instance would have fewer and fewer of the "old" queries warmed automatically.

      1. SOLR-571.patch
        26 kB
        Hoss Man
      2. SOLR-571.patch
        19 kB
        Tomás Fernández Löbbe

        Issue Links

          Activity

          Hide
          Yonik Seeley added a comment -

          Yep, great idea, and should be fairly simple to implement.

          Show
          Yonik Seeley added a comment - Yep, great idea, and should be fairly simple to implement.
          Hide
          Noble Paul added a comment -

          if someone has a large number of unique queries, will it not take too long to commit.

          Show
          Noble Paul added a comment - if someone has a large number of unique queries, will it not take too long to commit.
          Hide
          Tomás Fernández Löbbe added a comment -

          I don't understand why it could take longer to commit than when using an absolute value instead of a percentage.
          The number of items sent from the old cache to the new cache will depend on the size of the old cache and on the configured percentage. If someone uses a big percentage (like 75% or 100%) on a big cache, it might take long, but it will be pretty much the same of using a very big number on autowarmCount attribute on the same cache.
          Am I missing something? I can't see a problem here.

          Show
          Tomás Fernández Löbbe added a comment - I don't understand why it could take longer to commit than when using an absolute value instead of a percentage. The number of items sent from the old cache to the new cache will depend on the size of the old cache and on the configured percentage. If someone uses a big percentage (like 75% or 100%) on a big cache, it might take long, but it will be pretty much the same of using a very big number on autowarmCount attribute on the same cache. Am I missing something? I can't see a problem here.
          Hide
          Hoss Man added a comment -

          I can't see a problem here.

          me neither.

          Noble: the percentages will be evaluated relative the current size of the cache – for a full cache, setting to 100% won't be any slower then setting autowarmCount == size.

          what this changes is that people can set autowarmCount="50%" and then if the box stops serving queries, there will be a gradualdrop off in the number of items inserted into hte cache due to autowarming after each successive commit

          Show
          Hoss Man added a comment - I can't see a problem here. me neither. Noble: the percentages will be evaluated relative the current size of the cache – for a full cache, setting to 100% won't be any slower then setting autowarmCount == size. what this changes is that people can set autowarmCount="50%" and then if the box stops serving queries, there will be a gradualdrop off in the number of items inserted into hte cache due to autowarming after each successive commit
          Hide
          Noble Paul added a comment -

          S\o 50%is the 50% of cache size. so it should be finite.
          OK got it

          Show
          Noble Paul added a comment - S\o 50%is the 50% of cache size. so it should be finite. OK got it
          Hide
          Tomás Fernández Löbbe added a comment -

          On the attached patch, I modified classes LRUCache and FastLRUCache adding the capability of using a percentage value in the "autowarmCount" parameter. The percentage value gets evaluated with the current size of the "old" cache when warming the new cache.
          I also added some tests of the new functionality.

          Show
          Tomás Fernández Löbbe added a comment - On the attached patch, I modified classes LRUCache and FastLRUCache adding the capability of using a percentage value in the "autowarmCount" parameter. The percentage value gets evaluated with the current size of the "old" cache when warming the new cache. I also added some tests of the new functionality.
          Hide
          Hoss Man added a comment -

          Tomas: thanks for taking a crack at this, your patch (particularly the tests) are much appreciated!

          I'm not keen on proliferating the amount of cut/paste between LRUCache and FastLRUCache, so i refactored your new code into a baseclass. i also felt uncomfortable overloading the same int value to be both a number and a percentage depending on the value of a different boolean, so i also refactored that into an immutable reference class that abstracts the logic.

          revised patch attached, i'll commit as soon as i finish running all the tests

          Show
          Hoss Man added a comment - Tomas: thanks for taking a crack at this, your patch (particularly the tests) are much appreciated! I'm not keen on proliferating the amount of cut/paste between LRUCache and FastLRUCache, so i refactored your new code into a baseclass. i also felt uncomfortable overloading the same int value to be both a number and a percentage depending on the value of a different boolean, so i also refactored that into an immutable reference class that abstracts the logic. revised patch attached, i'll commit as soon as i finish running all the tests
          Hide
          Hoss Man added a comment -

          Committed revision 938708.

          Show
          Hoss Man added a comment - Committed revision 938708.
          Hide
          Tomás Fernández Löbbe added a comment -

          Excellent. Since this was my first patch for Solr, I didn't want to modify too much code just for a little functional change. Next time I'll feel freer to do it if it's necessary.

          Show
          Tomás Fernández Löbbe added a comment - Excellent. Since this was my first patch for Solr, I didn't want to modify too much code just for a little functional change. Next time I'll feel freer to do it if it's necessary.
          Hide
          Hoss Man added a comment -

          Since this was my first patch for Solr, I didn't want to modify too much code just for a little functional change.

          Your instincts were correct – patches should try to remain focused, so a major refactoring of the two classes would have confused the issue, but if you find yourself adding the same method to two different classes in a patch, refactoring it into a utility method to make it clear to the patch reader that the functionality really is identical is a good idea – it helps point the way to subsequent refactorings (like the super class i added).

          Show
          Hoss Man added a comment - Since this was my first patch for Solr, I didn't want to modify too much code just for a little functional change. Your instincts were correct – patches should try to remain focused, so a major refactoring of the two classes would have confused the issue, but if you find yourself adding the same method to two different classes in a patch, refactoring it into a utility method to make it clear to the patch reader that the functionality really is identical is a good idea – it helps point the way to subsequent refactorings (like the super class i added).
          Hide
          Hoss Man added a comment -

          Correcting Fix Version based on CHANGES.txt, see this thread for more details...

          http://mail-archives.apache.org/mod_mbox/lucene-dev/201005.mbox/%3Calpine.DEB.1.10.1005251052040.24672@radix.cryptio.net%3E

          Show
          Hoss Man added a comment - Correcting Fix Version based on CHANGES.txt, see this thread for more details... http://mail-archives.apache.org/mod_mbox/lucene-dev/201005.mbox/%3Calpine.DEB.1.10.1005251052040.24672@radix.cryptio.net%3E

            People

            • Assignee:
              Hoss Man
              Reporter:
              Hoss Man
            • Votes:
              1 Vote for this issue
              Watchers:
              1 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved:

                Development