Solr
  1. Solr
  2. SOLR-243

Create a hook to allow custom code to create custom IndexReaders

    Details

    • Type: Improvement Improvement
    • Status: Closed
    • Priority: Major Major
    • Resolution: Fixed
    • Affects Version/s: None
    • Fix Version/s: 1.4
    • Component/s: search
    • Labels:
      None
    • Environment:

      Solr core

      Description

      I have a customized IndexReader and I want to write a Solr plugin to use my derived IndexReader implementation. Currently IndexReader instantiation is hard coded to be:
      IndexReader.open(path)

      It would be really useful if this is done thru a plugable factory that can be configured, e.g. IndexReaderFactory

      interface IndexReaderFactory{
      IndexReader newReader(String name,String path);
      }

      the default implementation would just return: IndexReader.open(path)

      And in the newSearcher and getSearcher methods in SolrCore class can call the current factory implementation to get the IndexReader instance and then build the SolrIndexSearcher by passing in the reader.

      It would be really nice to add this improvement soon (This seems to be a trivial addition) as our project really depends on this.

      Thanks

      -John

      1. SOLR-243.patch
        27 kB
        Mark Miller
      2. SOLR-243.patch
        26 kB
        Mark Miller
      3. SOLR-243.patch
        28 kB
        Mark Miller
      4. SOLR-243.patch
        27 kB
        Mark Miller
      5. SOLR-243.patch
        12 kB
        Mark Miller
      6. SOLR-243.patch
        12 kB
        Mark Miller
      7. SOLR-243.patch
        12 kB
        Andrzej Bialecki
      8. SOLR-243.patch
        13 kB
        Mark Miller
      9. SOLR-243.patch
        16 kB
        Mark Miller
      10. SOLR-243.patch
        43 kB
        Mark Miller
      11. SOLR-243.patch
        15 kB
        Mark Miller
      12. indexReaderFactory.patch
        6 kB
        John Wang
      13. indexReaderFactory.patch
        6 kB
        John Wang
      14. indexReaderFactory.patch
        7 kB
        John Wang
      15. indexReaderFactory.patch
        9 kB
        Hoss Man
      16. indexReaderFactory.patch
        9 kB
        Hoss Man
      17. indexReaderFactory.patch
        9 kB
        Hoss Man
      18. indexReaderFactory.patch
        10 kB
        Hoss Man

        Issue Links

          Activity

          Hide
          John Wang added a comment -

          I have attached a patch for this issue.

          Show
          John Wang added a comment - I have attached a patch for this issue.
          Hide
          Hoss Man added a comment -

          some comments after reading the patch (haven't run it yet) ...

          1) in the future, please use 2 spaces for indenting (and no tabs)

          2) the existing public constructors for SolrIndexSearcher are now private which is an API change that has to be carefully considered ... on top of that, they don't even use the new IndexReder factory at all (they could get it by asking the SolrCore - it's a singleton).

          3) instead of adding a new indexReaderFactory element directly to the solrconfig.xml, it would probably make sense to get it when parsing the mainIndex/indexDefaults blocks.

          4) StandardIndexReaderFactory" might be a better class name then "DefaultIndexReaderFactory"

          5) I don't think we really need IndexReaderFactory.DEFAULT (static final instances in interfaces never make sense to me, they are not part of the "interface") ... just let SolrCore have a hardcoded instance of to use if it can't find one in the config.

          6) people should be able to specify configuration options for their factories ... either using an init(NamedLIst) method (like RequestHandlers) or using an init(Map<String,String>) method (like Caches, TokenFilters, etc...)

          7) catching "Exception" when calling newInstance() is too broad. explicitly catch only the exceptions that are expected and warrant using the default factory, otherwise you might silently ignore a really serious problem. ... although frankly, if someone configures an explict IndexReader, and it can't be instantiated, that should probably be SEVERE not just WARNING.

          Show
          Hoss Man added a comment - some comments after reading the patch (haven't run it yet) ... 1) in the future, please use 2 spaces for indenting (and no tabs) 2) the existing public constructors for SolrIndexSearcher are now private which is an API change that has to be carefully considered ... on top of that, they don't even use the new IndexReder factory at all (they could get it by asking the SolrCore - it's a singleton). 3) instead of adding a new indexReaderFactory element directly to the solrconfig.xml, it would probably make sense to get it when parsing the mainIndex/indexDefaults blocks. 4) StandardIndexReaderFactory" might be a better class name then "DefaultIndexReaderFactory" 5) I don't think we really need IndexReaderFactory.DEFAULT (static final instances in interfaces never make sense to me, they are not part of the "interface") ... just let SolrCore have a hardcoded instance of to use if it can't find one in the config. 6) people should be able to specify configuration options for their factories ... either using an init(NamedLIst) method (like RequestHandlers) or using an init(Map<String,String>) method (like Caches, TokenFilters, etc...) 7) catching "Exception" when calling newInstance() is too broad. explicitly catch only the exceptions that are expected and warrant using the default factory, otherwise you might silently ignore a really serious problem. ... although frankly, if someone configures an explict IndexReader, and it can't be instantiated, that should probably be SEVERE not just WARNING.
          Hide
          John Wang added a comment -

          1) Sorry about this, my eclipse IDE showed 2 spaces. I will remember in the future.

          2) Sorry about this one as well, I changed them from public to private to do some testing and forgot to change them back. I have no problem with them being public.

          3) That sounds good. I just picked a spot in solrConfig.xml

          4) Sure.

          5) I think that is a coding style, but I am not religious about it.

          6) That is a great idea! I wanted to add this but wasn't sure how to do it.

          7) Ok.

          Thanks for the code review!

          -John

          Show
          John Wang added a comment - 1) Sorry about this, my eclipse IDE showed 2 spaces. I will remember in the future. 2) Sorry about this one as well, I changed them from public to private to do some testing and forgot to change them back. I have no problem with them being public. 3) That sounds good. I just picked a spot in solrConfig.xml 4) Sure. 5) I think that is a coding style, but I am not religious about it. 6) That is a great idea! I wanted to add this but wasn't sure how to do it. 7) Ok. Thanks for the code review! -John
          Hide
          Hoss Man added a comment -

          regarding the IndexReaderFactory.DEFAULT, i have ahard time viewing it as good coding style at all ..but then again i've never seen any literature arguming for or against the concept, so i'm not sure what the "pros" are. My personal list of "cons"...
          a) violates the API / impl abstraction; interfaces shouldn't have to know about any of their Impls
          b) forces people who want to write their own impl to have the "default" class loaded at run time even if they don't use it.
          c) can't be changed; static variables in interfaces are be final (even if they aren't declared final) and can give the false impression to people reading an API that they can change it to something else.

          as far as #6 above (an init method) SolrCore.parseListener is a good example of how to NamedList style initing (see the SolrEventListener API). IndexSchema.readTokenFilterFactory is a good example of the simpler Map style initing.

          I saw an idea thrown around recently about making super interfaces for each of the two types of initialization so a lot of this code could be refactored away ... but that probably won't happen soon, and when/if it does it will be a big undertaking that will clean up everything, so in the mean time a cut/paste/tweak approach would be the best way to proceed

          Show
          Hoss Man added a comment - regarding the IndexReaderFactory.DEFAULT, i have ahard time viewing it as good coding style at all ..but then again i've never seen any literature arguming for or against the concept, so i'm not sure what the "pros" are. My personal list of "cons"... a) violates the API / impl abstraction; interfaces shouldn't have to know about any of their Impls b) forces people who want to write their own impl to have the "default" class loaded at run time even if they don't use it. c) can't be changed; static variables in interfaces are be final (even if they aren't declared final) and can give the false impression to people reading an API that they can change it to something else. as far as #6 above (an init method) SolrCore.parseListener is a good example of how to NamedList style initing (see the SolrEventListener API). IndexSchema.readTokenFilterFactory is a good example of the simpler Map style initing. I saw an idea thrown around recently about making super interfaces for each of the two types of initialization so a lot of this code could be refactored away ... but that probably won't happen soon, and when/if it does it will be a big undertaking that will clean up everything, so in the mean time a cut/paste/tweak approach would be the best way to proceed
          Hide
          John Wang added a comment -

          New patch incorporating Chris's comments 1-6.

          for item 3) config is now placed under indexDefaults section.

          Show
          John Wang added a comment - New patch incorporating Chris's comments 1-6. for item 3) config is now placed under indexDefaults section.
          Hide
          Yonik Seeley added a comment -

          Thanks John, I also have a need for this type of functionality.
          In my case, I need to open open a MultiReader over several index segments.

          One thing that crosses my mind is if some factories will need a little more context, such as what reader is being created (for what purpose). For example, is it the main index reader, or is it for something like the spelling index, is it only for deleting in the IndexWriter, etc.

          When instantiating the factory, why not just call Config.newInstance(name) and let it throw an exception (or did you want to continue on after a failure to find or instantiate?)

          Show
          Yonik Seeley added a comment - Thanks John, I also have a need for this type of functionality. In my case, I need to open open a MultiReader over several index segments. One thing that crosses my mind is if some factories will need a little more context, such as what reader is being created (for what purpose). For example, is it the main index reader, or is it for something like the spelling index, is it only for deleting in the IndexWriter, etc. When instantiating the factory, why not just call Config.newInstance(name) and let it throw an exception (or did you want to continue on after a failure to find or instantiate?)
          Hide
          Yonik Seeley added a comment -

          Minor nit: is there a reason that IndexReaderFactory can't be a class instead of an interface?
          It doesn't seem to be a likely candidate for multiple inheritance, and making it a class allows us to "upgrade" the interface in the future while providing a backward compatible default implementation for people who have already implemented it.

          Show
          Yonik Seeley added a comment - Minor nit: is there a reason that IndexReaderFactory can't be a class instead of an interface? It doesn't seem to be a likely candidate for multiple inheritance, and making it a class allows us to "upgrade" the interface in the future while providing a backward compatible default implementation for people who have already implemented it.
          Hide
          John Wang added a comment -

          Hi Yonik:

          1) context in factory: I am not sure how the interface would look like, suggestions?

          2) I catch the exception to default to the normal IndexReader.open mechanism.

          3) I just picked it to be an interface, I think making it an abstract class is fine.

          thanks

          -John

          Show
          John Wang added a comment - Hi Yonik: 1) context in factory: I am not sure how the interface would look like, suggestions? 2) I catch the exception to default to the normal IndexReader.open mechanism. 3) I just picked it to be an interface, I think making it an abstract class is fine. thanks -John
          Hide
          John Wang added a comment -

          My motivation in doing this is to create a plugin into solr for my project:

          www.browseengine.com
          or
          bobo-browse.sourceforge.net

          which is a more extensive facetted search implementation. I need a special IndexReader to do the plugin.

          Please take a look.

          Thanks

          -John

          Show
          John Wang added a comment - My motivation in doing this is to create a plugin into solr for my project: www.browseengine.com or bobo-browse.sourceforge.net which is a more extensive facetted search implementation. I need a special IndexReader to do the plugin. Please take a look. Thanks -John
          Hide
          Hoss Man added a comment -

          comments on the latest patch..

          1) SolrIndexSeacher constructor is still private

          2) i believe Yonik's question about calling Config.newInstance(name) was in realtion to initializing the Factory (you currently just use "factory = clazz.newInstance()")

          3) my comment about the mainIndex/indexDefaults blocks was that for many options they can be specified in the indexDefaults section, and overriden in the mainIndex section ... from what i can tell the current patch requires it to be in indexDefaults, ideally it would follow the same behavior as the other settings (mainIndex is used for the .. well, "main index" while indexDefaults also applies to other intermediate indexes ... down the road we may deprecate it, or expand it's usage for other types of micro indexes various pieces of functionality use like spellchecking) but for now any option relating to an indexreader/writer should support the "dfaulting mechanism and work with both sections.

          (I'm not sure what type of API Yonik might have been thinking of for conveying context).

          Show
          Hoss Man added a comment - comments on the latest patch.. 1) SolrIndexSeacher constructor is still private 2) i believe Yonik's question about calling Config.newInstance(name) was in realtion to initializing the Factory (you currently just use "factory = clazz.newInstance()") 3) my comment about the mainIndex/indexDefaults blocks was that for many options they can be specified in the indexDefaults section, and overriden in the mainIndex section ... from what i can tell the current patch requires it to be in indexDefaults, ideally it would follow the same behavior as the other settings (mainIndex is used for the .. well, "main index" while indexDefaults also applies to other intermediate indexes ... down the road we may deprecate it, or expand it's usage for other types of micro indexes various pieces of functionality use like spellchecking) but for now any option relating to an indexreader/writer should support the "dfaulting mechanism and work with both sections. (I'm not sure what type of API Yonik might have been thinking of for conveying context).
          Hide
          John Wang added a comment -

          Are you looking at the right patch?

          For 1) at least, they are public.

          2) I am using Config.findClass

          3) I just picked one from your suggestions, mainIndex is fine.

          I feel I am jumping through quite a few arbitrary hoops just to contribute to an open source project, and this is wrong and defeats the whole purpose of the idea of open source.

          I think being more constructive and result oriented instead of being difficult and purist can benefit this project much more.

          Show
          John Wang added a comment - Are you looking at the right patch? For 1) at least, they are public. 2) I am using Config.findClass 3) I just picked one from your suggestions, mainIndex is fine. I feel I am jumping through quite a few arbitrary hoops just to contribute to an open source project, and this is wrong and defeats the whole purpose of the idea of open source. I think being more constructive and result oriented instead of being difficult and purist can benefit this project much more.
          Hide
          Hoss Man added a comment -

          1) i'm sorry, i transposed the lines in my mind when i was readingthe patch (you've made a private constructor public, not the otherway arround – my mistake)

          2) yes, you're using Config.findClass ... what yonik asked was if there was a particular reason not to use Config.newInstance(name) in the loadIndexReaderFactory ... there is a lot of duplicate code in that method (mainly exception handling) that Config.newInstance takes care of for you.

          3) I think you're missing my point about indexDefaults and mainIndex ... it's not a matter of just picking one, it's making it work with both so that a factory can be specified in the defaults for use anytime an IndexReader is opened, or from mainIndex awhen the "main index" is opened. I just poked around and found that the relevant class is "SolrIndexConfig" ... my suggestion was that this be where the IndexReaderFactory hook be so that it works the same way.

          I'm sorry if you feel like you are jumping through a lot of hoops ... it's not my intention to be difficult, i'm just making comments on the patch and asking general questions (not specificly directed at your patch) about how Solr as a project can best support the topic of this issue (hooks to allow custom code to create custom index readers).

          If the patch you have works well for you that's great, but that doesn't mean it will work well for everyone, which is something committers have to keep that in mind ... making public API changes (including new config syntax and especially new plugin hooks) is a serious change to the project and has to be considered very carefully because we have to be able to support it for a very very long time.

          Show
          Hoss Man added a comment - 1) i'm sorry, i transposed the lines in my mind when i was readingthe patch (you've made a private constructor public, not the otherway arround – my mistake) 2) yes, you're using Config.findClass ... what yonik asked was if there was a particular reason not to use Config.newInstance(name) in the loadIndexReaderFactory ... there is a lot of duplicate code in that method (mainly exception handling) that Config.newInstance takes care of for you. 3) I think you're missing my point about indexDefaults and mainIndex ... it's not a matter of just picking one, it's making it work with both so that a factory can be specified in the defaults for use anytime an IndexReader is opened, or from mainIndex awhen the "main index" is opened. I just poked around and found that the relevant class is "SolrIndexConfig" ... my suggestion was that this be where the IndexReaderFactory hook be so that it works the same way. I'm sorry if you feel like you are jumping through a lot of hoops ... it's not my intention to be difficult, i'm just making comments on the patch and asking general questions (not specificly directed at your patch) about how Solr as a project can best support the topic of this issue (hooks to allow custom code to create custom index readers). If the patch you have works well for you that's great, but that doesn't mean it will work well for everyone, which is something committers have to keep that in mind ... making public API changes (including new config syntax and especially new plugin hooks) is a serious change to the project and has to be considered very carefully because we have to be able to support it for a very very long time.
          Hide
          John Wang added a comment -

          My apologies for not being patient with this process.
          I have made the requested changes and submitted another patch.

          Please let me know if these are the correct things to do.

          Thanks

          -John

          Show
          John Wang added a comment - My apologies for not being patient with this process. I have made the requested changes and submitted another patch. Please let me know if these are the correct things to do. Thanks -John
          Hide
          Kevin Osborn added a comment -

          We used this patch with great success. We have a custom multiindex reader. This patch allowed us to just plug in our changes without resorting to hack up the base Solr code.

          Show
          Kevin Osborn added a comment - We used this patch with great success. We have a custom multiindex reader. This patch allowed us to just plug in our changes without resorting to hack up the base Solr code.
          Hide
          John Wang added a comment -

          Thanks Kevin!
          Please vote on this patch to have it committed.

          -John

          Show
          John Wang added a comment - Thanks Kevin! Please vote on this patch to have it committed. -John
          Hide
          John Wang added a comment -

          What is the plan for this patch?
          We have a couple of votes for it.

          Thanks

          -John

          Show
          John Wang added a comment - What is the plan for this patch? We have a couple of votes for it. Thanks -John
          Hide
          Hoss Man added a comment -

          minimal set of changes i could make to previous patch so that: a) it compiles against the trunk r636903; b) it doesn't rely on the deprecated singletons (SolrConfig or SolrCore)

          Show
          Hoss Man added a comment - minimal set of changes i could make to previous patch so that: a) it compiles against the trunk r636903; b) it doesn't rely on the deprecated singletons (SolrConfig or SolrCore)
          Hide
          Hoss Man added a comment -

          further refinement: made IndexReaderFactory an abstract class extending NamedListInitializedPlugin.

          Show
          Hoss Man added a comment - further refinement: made IndexReaderFactory an abstract class extending NamedListInitializedPlugin.
          Hide
          Hoss Man added a comment -

          John: first off, i want to apologize for the extremely belated patch review ... I know i told you i'd try to look at this months ago, but ... yeah ... life tends to get in the way of patches.

          on the whole, things seem pretty good – although you were still using an interface instead of an abstract class. Having the main API for a type of plugin be an abstract class is an important mechanism to help future proof ourselves against possible additions we want to make the the plugin API in the future; if it's an interface we can't really change it once it's released (because we might break people Impls); if it's an abstract class, we can always provide a default impl for people.

          the biggest concern i have about this patch at the moment is that there are no tests, and no example configs, so it's hard to be sure it's even working at all

          here's what i see as the current todo list for this issue...

          1) there are some legacy SolrIndexSearcher constructors that need to delegate to the SolrCore to get indexReaderFactory ... perhaps we should have a helper method that decides which SolrIndexConfig to use based on the "name" ?
          2) we need tests showing custom IndexReaderFactory getting used (even if it's just a mock IndexReaderFactory thatsets a boolean to show it's being used) ... this will also serve as a test that config syntax works.
          3) need commented out example of using a custom indexReaderFactory in example/solr/conf/solrconfig.xml
          4) sanity check this against SOLR-465, make sure we aren't painting ourselves into a corner.
          5) we should make IndexReaderFactory use the AbstractPluginLoader stuff and remove the guts of SolrIndexConfig.loadIndexReaderFactory. It looks like we'll need to add a "single item" version of load to the AbstractPluginLoader to make that work well.

          #5 is something that can be done after this is committed, but 1-4 are pretty important. If you can update the patch with some configs/tests i'll think about the legacy SolrIndexSearcher constructors and try to figure out a good solution for them

          Show
          Hoss Man added a comment - John: first off, i want to apologize for the extremely belated patch review ... I know i told you i'd try to look at this months ago, but ... yeah ... life tends to get in the way of patches. on the whole, things seem pretty good – although you were still using an interface instead of an abstract class. Having the main API for a type of plugin be an abstract class is an important mechanism to help future proof ourselves against possible additions we want to make the the plugin API in the future; if it's an interface we can't really change it once it's released (because we might break people Impls); if it's an abstract class, we can always provide a default impl for people. the biggest concern i have about this patch at the moment is that there are no tests, and no example configs, so it's hard to be sure it's even working at all here's what i see as the current todo list for this issue... 1) there are some legacy SolrIndexSearcher constructors that need to delegate to the SolrCore to get indexReaderFactory ... perhaps we should have a helper method that decides which SolrIndexConfig to use based on the "name" ? 2) we need tests showing custom IndexReaderFactory getting used (even if it's just a mock IndexReaderFactory thatsets a boolean to show it's being used) ... this will also serve as a test that config syntax works. 3) need commented out example of using a custom indexReaderFactory in example/solr/conf/solrconfig.xml 4) sanity check this against SOLR-465 , make sure we aren't painting ourselves into a corner. 5) we should make IndexReaderFactory use the AbstractPluginLoader stuff and remove the guts of SolrIndexConfig.loadIndexReaderFactory. It looks like we'll need to add a "single item" version of load to the AbstractPluginLoader to make that work well. #5 is something that can be done after this is committed, but 1-4 are pretty important. If you can update the patch with some configs/tests i'll think about the legacy SolrIndexSearcher constructors and try to figure out a good solution for them
          Hide
          Hoss Man added a comment -

          Grrr.... reattaching with the right radio button this time.

          further refinement: made IndexReaderFactory an abstract class extending NamedListInitializedPlugin.

          Show
          Hoss Man added a comment - Grrr.... reattaching with the right radio button this time. further refinement: made IndexReaderFactory an abstract class extending NamedListInitializedPlugin.
          Hide
          Hoss Man added a comment -

          thinking about the legacy SolrIndexSearch constructors a bit more, and how SolrIndexConfig is used, i realized i few things...

          • we can just always use the mainIndex config in those constructors
          • we should really deprecate one of those constructors anyway
          • we don't need to be quite as paranoid about getting the defaultIndexConfig if main isn't set – SolrIndexConfig takes care of that for us.
          • there were some tabs still left in the diff.
          Show
          Hoss Man added a comment - thinking about the legacy SolrIndexSearch constructors a bit more, and how SolrIndexConfig is used, i realized i few things... we can just always use the mainIndex config in those constructors we should really deprecate one of those constructors anyway we don't need to be quite as paranoid about getting the defaultIndexConfig if main isn't set – SolrIndexConfig takes care of that for us. there were some tabs still left in the diff.
          Hide
          Mike Klaas added a comment -

          Do we still want to target 1.3 here? (Seems like there is a lot to do before it is commit-worthy, based on the comments)

          Show
          Mike Klaas added a comment - Do we still want to target 1.3 here? (Seems like there is a lot to do before it is commit-worthy, based on the comments)
          Hide
          John Wang added a comment -

          Not sure what do you mean?It was supposed to be a simple addition.

          -John

          Show
          John Wang added a comment - Not sure what do you mean?It was supposed to be a simple addition. -John
          Hide
          Mike Klaas added a comment -

          Hi John,

          Hoss has marked the issue for 1.3, so it will be in the release.

          -Mike

          Show
          Mike Klaas added a comment - Hi John, Hoss has marked the issue for 1.3, so it will be in the release. -Mike
          Hide
          Hoss Man added a comment -

          Hoss has marked the issue for 1.3, so it will be in the release.

          for the record, i marked it as 1.3 because itwould be nice to see this in 1.3 ... but as i said in my 2008-03-13 comment: we need unit tests and example configuration before i'm willing to commit.

          Show
          Hoss Man added a comment - Hoss has marked the issue for 1.3, so it will be in the release. for the record, i marked it as 1.3 because itwould be nice to see this in 1.3 ... but as i said in my 2008-03-13 comment: we need unit tests and example configuration before i'm willing to commit.
          Hide
          John Wang added a comment -

          Sorry, I didn't see Hoss's earlier comments.
          Thanks

          -John

          Show
          John Wang added a comment - Sorry, I didn't see Hoss's earlier comments. Thanks -John
          Hide
          Hoss Man added a comment -

          unmarking for 1.3 ... on hold until we have tests and/or more interest

          Show
          Hoss Man added a comment - unmarking for 1.3 ... on hold until we have tests and/or more interest
          Hide
          Mark Miller added a comment -

          Okay, I am after SOLR-465, but since this guy hovers around it, so:

          New patch with a test to be sure the config / alt factory is indeed used. Factory class loaded in SolrCore now with the pluginloader - added a new loadSingle method. SolrIndexConfig really doesn't seem like the right place for that method that was there anyway...

          So, in pushing SOLR-465, I'd like to do things pretty much as is over there (except use the pluginloader stuff) just before loading the reader factory, so that the correct Directory imp can be passed to the getReader method. So comments to how this is done here will direct what I attempt there.

          Thoughts? Pick away please, I'd like to help out wrapping this and 465 soon.

          Show
          Mark Miller added a comment - Okay, I am after SOLR-465 , but since this guy hovers around it, so: New patch with a test to be sure the config / alt factory is indeed used. Factory class loaded in SolrCore now with the pluginloader - added a new loadSingle method. SolrIndexConfig really doesn't seem like the right place for that method that was there anyway... So, in pushing SOLR-465 , I'd like to do things pretty much as is over there (except use the pluginloader stuff) just before loading the reader factory, so that the correct Directory imp can be passed to the getReader method. So comments to how this is done here will direct what I attempt there. Thoughts? Pick away please, I'd like to help out wrapping this and 465 soon.
          Hide
          Mark Miller added a comment -

          Updates patch to trunk

          Show
          Mark Miller added a comment - Updates patch to trunk
          Hide
          Mark Miller added a comment -

          Adds a config option to turn on/off reopening readers. apache pork.

          Show
          Mark Miller added a comment - Adds a config option to turn on/off reopening readers. apache pork.
          Hide
          Mark Miller added a comment -

          First attempt at getting this back up to trunk.

          Should probably merge the alt dir and alt reader test config files into one.

          Show
          Mark Miller added a comment - First attempt at getting this back up to trunk. Should probably merge the alt dir and alt reader test config files into one.
          Hide
          Shalin Shekhar Mangar added a comment -

          Mark, are you still interested in this issue? If not, should we mark this to 1.5?

          Show
          Shalin Shekhar Mangar added a comment - Mark, are you still interested in this issue? If not, should we mark this to 1.5?
          Hide
          Mark Miller added a comment -

          Updated to trunk and now uses alt directory test config file rather than a new one.

          Show
          Mark Miller added a comment - Updated to trunk and now uses alt directory test config file rather than a new one.
          Hide
          Shalin Shekhar Mangar added a comment -

          Marked for 1.5

          Show
          Shalin Shekhar Mangar added a comment - Marked for 1.5
          Hide
          Mark Miller added a comment -

          to trunk

          Show
          Mark Miller added a comment - to trunk
          Hide
          Andrzej Bialecki added a comment -

          This is a useful functionality when using FilterIndexReader or ParallelReader, +1 for adding it to core. I updated the patch to the latest trunk - all tests pass.

          Show
          Andrzej Bialecki added a comment - This is a useful functionality when using FilterIndexReader or ParallelReader, +1 for adding it to core. I updated the patch to the latest trunk - all tests pass.
          Hide
          Mark Miller added a comment -

          I'm going to commit this if there are no objections.

          • Mark
          Show
          Mark Miller added a comment - I'm going to commit this if there are no objections. Mark
          Hide
          Shalin Shekhar Mangar added a comment -

          Mark, this patch still has the code for SOLR-1184.

          Show
          Shalin Shekhar Mangar added a comment - Mark, this patch still has the code for SOLR-1184 .
          Hide
          Mark Miller added a comment -

          Yup I know - in the case that that goes in first, its easier to resolve (I had already put it here). And if it does not, I'll remove it before committing.

          Show
          Mark Miller added a comment - Yup I know - in the case that that goes in first, its easier to resolve (I had already put it here). And if it does not, I'll remove it before committing.
          Hide
          Mark Miller added a comment -

          to trunk

          Show
          Mark Miller added a comment - to trunk
          Hide
          Mark Miller added a comment -

          to trunk, remove non readonly reader factory method and fixes readOnly propagation in a couple spots.

          Show
          Mark Miller added a comment - to trunk, remove non readonly reader factory method and fixes readOnly propagation in a couple spots.
          Hide
          Mark Miller added a comment -

          Committed, thanks to all involved.

          Show
          Mark Miller added a comment - Committed, thanks to all involved.
          Hide
          Grant Ingersoll added a comment -

          Bulk close for Solr 1.4

          Show
          Grant Ingersoll added a comment - Bulk close for Solr 1.4

            People

            • Assignee:
              Mark Miller
              Reporter:
              John Wang
            • Votes:
              6 Vote for this issue
              Watchers:
              2 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved:

                Development