[jira] [Updated] (LUCENE-4132) IndexWriterConfig live settings
[ https://issues.apache.org/jira/browse/LUCENE-4132?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ] Shai Erera updated LUCENE-4132: --- Attachment: LUCENE-4132.patch Patch removes Cloenable from LiveIWC. Now only IWC is Cloenable. > IndexWriterConfig live settings > --- > > Key: LUCENE-4132 > URL: https://issues.apache.org/jira/browse/LUCENE-4132 > Project: Lucene - Java > Issue Type: Improvement >Reporter: Shai Erera >Assignee: Shai Erera >Priority: Minor > Fix For: 4.0, 5.0 > > Attachments: LUCENE-4132.patch, LUCENE-4132.patch, LUCENE-4132.patch, > LUCENE-4132.patch, LUCENE-4132.patch, LUCENE-4132.patch, LUCENE-4132.patch > > > A while ago there was a discussion about making some IW settings "live" and I > remember that RAM buffer size was one of them. Judging from IW code, I see > that RAM buffer can be changed "live" as IW never caches it. > However, I don't remember which other settings were decided to be "live" and > I don't see any documentation in IW nor IWC for that. IW.getConfig mentions: > {code} > * NOTE: some settings may be changed on the > * returned {@link IndexWriterConfig}, and will take > * effect in the current IndexWriter instance. See the > * javadocs for the specific setters in {@link > * IndexWriterConfig} for details. > {code} > But there's no text on e.g. IWC.setRAMBuffer mentioning that. > I think that it'd be good if we make it easier for users to tell which of the > settings are "live" ones. There are few possible ways to do it: > * Introduce a custom @live.setting tag on the relevant IWC.set methods, and > add special text for them in build.xml > ** Or, drop the tag and just document it clearly. > * Separate IWC to two interfaces, LiveConfig and OneTimeConfig (name > proposals are welcome !), have IWC impl both, and introduce another > IW.getLiveConfig which will return that interface, thereby clearly letting > the user know which of the settings are "live". > It'd be good if IWC itself could only expose setXYZ methods for the "live" > settings though. So perhaps, off the top of my head, we can do something like > this: > * Introduce a Config object, which is essentially what IWC is today, and pass > it to IW. > * IW will create a different object, IWC from that Config and IW.getConfig > will return IWC. > * IWC itself will only have setXYZ methods for the "live" settings. > It adds another object, but user code doesn't change - it still creates a > Config object when initializing IW, and need to handle a different type if it > ever calls IW.getConfig. > Maybe that's not such a bad idea? -- This message is automatically generated by JIRA. If you think it was sent incorrectly, please contact your JIRA administrators: https://issues.apache.org/jira/secure/ContactAdministrators!default.jspa For more information on JIRA, see: http://www.atlassian.com/software/jira - To unsubscribe, e-mail: dev-unsubscr...@lucene.apache.org For additional commands, e-mail: dev-h...@lucene.apache.org
[jira] [Updated] (LUCENE-4132) IndexWriterConfig live settings
[ https://issues.apache.org/jira/browse/LUCENE-4132?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ] Shai Erera updated LUCENE-4132: --- Attachment: LUCENE-4132.patch Good catch Mike ! It went away in the last changes. I re-added testReuse, with asserting that e.g. the MP instances returned from LiveIWC are not the same. > IndexWriterConfig live settings > --- > > Key: LUCENE-4132 > URL: https://issues.apache.org/jira/browse/LUCENE-4132 > Project: Lucene - Java > Issue Type: Improvement >Reporter: Shai Erera >Assignee: Shai Erera >Priority: Minor > Fix For: 4.0, 5.0 > > Attachments: LUCENE-4132.patch, LUCENE-4132.patch, LUCENE-4132.patch, > LUCENE-4132.patch, LUCENE-4132.patch, LUCENE-4132.patch > > > A while ago there was a discussion about making some IW settings "live" and I > remember that RAM buffer size was one of them. Judging from IW code, I see > that RAM buffer can be changed "live" as IW never caches it. > However, I don't remember which other settings were decided to be "live" and > I don't see any documentation in IW nor IWC for that. IW.getConfig mentions: > {code} > * NOTE: some settings may be changed on the > * returned {@link IndexWriterConfig}, and will take > * effect in the current IndexWriter instance. See the > * javadocs for the specific setters in {@link > * IndexWriterConfig} for details. > {code} > But there's no text on e.g. IWC.setRAMBuffer mentioning that. > I think that it'd be good if we make it easier for users to tell which of the > settings are "live" ones. There are few possible ways to do it: > * Introduce a custom @live.setting tag on the relevant IWC.set methods, and > add special text for them in build.xml > ** Or, drop the tag and just document it clearly. > * Separate IWC to two interfaces, LiveConfig and OneTimeConfig (name > proposals are welcome !), have IWC impl both, and introduce another > IW.getLiveConfig which will return that interface, thereby clearly letting > the user know which of the settings are "live". > It'd be good if IWC itself could only expose setXYZ methods for the "live" > settings though. So perhaps, off the top of my head, we can do something like > this: > * Introduce a Config object, which is essentially what IWC is today, and pass > it to IW. > * IW will create a different object, IWC from that Config and IW.getConfig > will return IWC. > * IWC itself will only have setXYZ methods for the "live" settings. > It adds another object, but user code doesn't change - it still creates a > Config object when initializing IW, and need to handle a different type if it > ever calls IW.getConfig. > Maybe that's not such a bad idea? -- This message is automatically generated by JIRA. If you think it was sent incorrectly, please contact your JIRA administrators: https://issues.apache.org/jira/secure/ContactAdministrators!default.jspa For more information on JIRA, see: http://www.atlassian.com/software/jira - To unsubscribe, e-mail: dev-unsubscr...@lucene.apache.org For additional commands, e-mail: dev-h...@lucene.apache.org
[jira] [Updated] (LUCENE-4132) IndexWriterConfig live settings
[ https://issues.apache.org/jira/browse/LUCENE-4132?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ] Shai Erera updated LUCENE-4132: --- Attachment: LUCENE-4132.patch bq. Can we override all methods so the javadocs aren't confusing. Good idea! Done bq. Also can we rename it to LiveIndexWriterConfig? Done > IndexWriterConfig live settings > --- > > Key: LUCENE-4132 > URL: https://issues.apache.org/jira/browse/LUCENE-4132 > Project: Lucene - Java > Issue Type: Improvement >Reporter: Shai Erera >Priority: Minor > Fix For: 4.0, 5.0 > > Attachments: LUCENE-4132.patch, LUCENE-4132.patch, LUCENE-4132.patch, > LUCENE-4132.patch, LUCENE-4132.patch > > > A while ago there was a discussion about making some IW settings "live" and I > remember that RAM buffer size was one of them. Judging from IW code, I see > that RAM buffer can be changed "live" as IW never caches it. > However, I don't remember which other settings were decided to be "live" and > I don't see any documentation in IW nor IWC for that. IW.getConfig mentions: > {code} > * NOTE: some settings may be changed on the > * returned {@link IndexWriterConfig}, and will take > * effect in the current IndexWriter instance. See the > * javadocs for the specific setters in {@link > * IndexWriterConfig} for details. > {code} > But there's no text on e.g. IWC.setRAMBuffer mentioning that. > I think that it'd be good if we make it easier for users to tell which of the > settings are "live" ones. There are few possible ways to do it: > * Introduce a custom @live.setting tag on the relevant IWC.set methods, and > add special text for them in build.xml > ** Or, drop the tag and just document it clearly. > * Separate IWC to two interfaces, LiveConfig and OneTimeConfig (name > proposals are welcome !), have IWC impl both, and introduce another > IW.getLiveConfig which will return that interface, thereby clearly letting > the user know which of the settings are "live". > It'd be good if IWC itself could only expose setXYZ methods for the "live" > settings though. So perhaps, off the top of my head, we can do something like > this: > * Introduce a Config object, which is essentially what IWC is today, and pass > it to IW. > * IW will create a different object, IWC from that Config and IW.getConfig > will return IWC. > * IWC itself will only have setXYZ methods for the "live" settings. > It adds another object, but user code doesn't change - it still creates a > Config object when initializing IW, and need to handle a different type if it > ever calls IW.getConfig. > Maybe that's not such a bad idea? -- This message is automatically generated by JIRA. If you think it was sent incorrectly, please contact your JIRA administrators: https://issues.apache.org/jira/secure/ContactAdministrators!default.jspa For more information on JIRA, see: http://www.atlassian.com/software/jira - To unsubscribe, e-mail: dev-unsubscr...@lucene.apache.org For additional commands, e-mail: dev-h...@lucene.apache.org
[jira] [Updated] (LUCENE-4132) IndexWriterConfig live settings
[ https://issues.apache.org/jira/browse/LUCENE-4132?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ] Shai Erera updated LUCENE-4132: --- Attachment: LUCENE-4132.patch Thanks Uwe. The test is now fixed by saving all 'synthetic' methods and all 'setter' methods and verifying in the end that all of them were received from IWC too. > IndexWriterConfig live settings > --- > > Key: LUCENE-4132 > URL: https://issues.apache.org/jira/browse/LUCENE-4132 > Project: Lucene - Java > Issue Type: Improvement >Reporter: Shai Erera >Priority: Minor > Fix For: 4.0, 5.0 > > Attachments: LUCENE-4132.patch, LUCENE-4132.patch, LUCENE-4132.patch, > LUCENE-4132.patch > > > A while ago there was a discussion about making some IW settings "live" and I > remember that RAM buffer size was one of them. Judging from IW code, I see > that RAM buffer can be changed "live" as IW never caches it. > However, I don't remember which other settings were decided to be "live" and > I don't see any documentation in IW nor IWC for that. IW.getConfig mentions: > {code} > * NOTE: some settings may be changed on the > * returned {@link IndexWriterConfig}, and will take > * effect in the current IndexWriter instance. See the > * javadocs for the specific setters in {@link > * IndexWriterConfig} for details. > {code} > But there's no text on e.g. IWC.setRAMBuffer mentioning that. > I think that it'd be good if we make it easier for users to tell which of the > settings are "live" ones. There are few possible ways to do it: > * Introduce a custom @live.setting tag on the relevant IWC.set methods, and > add special text for them in build.xml > ** Or, drop the tag and just document it clearly. > * Separate IWC to two interfaces, LiveConfig and OneTimeConfig (name > proposals are welcome !), have IWC impl both, and introduce another > IW.getLiveConfig which will return that interface, thereby clearly letting > the user know which of the settings are "live". > It'd be good if IWC itself could only expose setXYZ methods for the "live" > settings though. So perhaps, off the top of my head, we can do something like > this: > * Introduce a Config object, which is essentially what IWC is today, and pass > it to IW. > * IW will create a different object, IWC from that Config and IW.getConfig > will return IWC. > * IWC itself will only have setXYZ methods for the "live" settings. > It adds another object, but user code doesn't change - it still creates a > Config object when initializing IW, and need to handle a different type if it > ever calls IW.getConfig. > Maybe that's not such a bad idea? -- This message is automatically generated by JIRA. If you think it was sent incorrectly, please contact your JIRA administrators: https://issues.apache.org/jira/secure/ContactAdministrators!default.jspa For more information on JIRA, see: http://www.atlassian.com/software/jira - To unsubscribe, e-mail: dev-unsubscr...@lucene.apache.org For additional commands, e-mail: dev-h...@lucene.apache.org
[jira] [Updated] (LUCENE-4132) IndexWriterConfig live settings
[ https://issues.apache.org/jira/browse/LUCENE-4132?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ] Shai Erera updated LUCENE-4132: --- Attachment: LUCENE-4132.patch Sorry if it came across like that, but I don't mean to rush or shove this issue in. I'm usually after consensus and I appreciate your feedback. I took another look at this, and found a solution without generics. Funny thing is, that's the first solution that came to my mind, but I guess at the time it didn't picture well enough, so I discarded it :). Now we have only LiveConfig and IndexWriterConfig, where IWC extends LC and overrides all setter methods. The "live" setters are overridden just to return IWC type, and call super.setXYZ(). So we don't have code dup, and whoever has IWC type at hand, will receive IWC back from all set() methods. LC is public class but with package-private ctors, one that takes IWC (used by IndexWriter) and one that takes Analyzer+Version, to match IWC's. It contains all "live" members as private, and the others as protected, so that IWC can set them. Since it cannot be sub-classed outside the package, this is 'safe'. The only thing that bothers me, and I'm not sure if it can be fixed, but this is not critical either, is TestIWC.testSettersChaining(). For some reason, even though I override the setters from LC in IWC, and set their return type to IWC, reflection still returns their return type as LiveConfig. This only affects the test, since if I do: {code} IndexWriterConfig conf; conf.setMaxBufferedDocs(); // or any other set from LC {code} the return type is IWC. If anyone knows how to solve it, please let me know, otherwise we'll just have to live with the modification to the test, and the chance that future "live" setters may be incorrectly overridden by IWC to not return IWC type That is not an error, just a convenience. Besides that, and if I follow your comments and concerns properly, I think this is now ready to commit -- there's no extra complexity (generics, 3 classes etc.), and with better compile time protection against misuse. > IndexWriterConfig live settings > --- > > Key: LUCENE-4132 > URL: https://issues.apache.org/jira/browse/LUCENE-4132 > Project: Lucene - Java > Issue Type: Improvement >Reporter: Shai Erera >Priority: Minor > Fix For: 4.0, 5.0 > > Attachments: LUCENE-4132.patch, LUCENE-4132.patch, LUCENE-4132.patch > > > A while ago there was a discussion about making some IW settings "live" and I > remember that RAM buffer size was one of them. Judging from IW code, I see > that RAM buffer can be changed "live" as IW never caches it. > However, I don't remember which other settings were decided to be "live" and > I don't see any documentation in IW nor IWC for that. IW.getConfig mentions: > {code} > * NOTE: some settings may be changed on the > * returned {@link IndexWriterConfig}, and will take > * effect in the current IndexWriter instance. See the > * javadocs for the specific setters in {@link > * IndexWriterConfig} for details. > {code} > But there's no text on e.g. IWC.setRAMBuffer mentioning that. > I think that it'd be good if we make it easier for users to tell which of the > settings are "live" ones. There are few possible ways to do it: > * Introduce a custom @live.setting tag on the relevant IWC.set methods, and > add special text for them in build.xml > ** Or, drop the tag and just document it clearly. > * Separate IWC to two interfaces, LiveConfig and OneTimeConfig (name > proposals are welcome !), have IWC impl both, and introduce another > IW.getLiveConfig which will return that interface, thereby clearly letting > the user know which of the settings are "live". > It'd be good if IWC itself could only expose setXYZ methods for the "live" > settings though. So perhaps, off the top of my head, we can do something like > this: > * Introduce a Config object, which is essentially what IWC is today, and pass > it to IW. > * IW will create a different object, IWC from that Config and IW.getConfig > will return IWC. > * IWC itself will only have setXYZ methods for the "live" settings. > It adds another object, but user code doesn't change - it still creates a > Config object when initializing IW, and need to handle a different type if it > ever calls IW.getConfig. > Maybe that's not such a bad idea? -- This message is automatically generated by JIRA. If you think it was sent incorrectly, please contact your JIRA administrators: https://issues.apache.org/jira/secure/ContactAdministrators!default.jspa For more information on JIRA, see: http://www.atlassian.com/software/jira - To unsubscribe, e-mail: dev-unsubscr...@lucene.apache.org For additional commands, e-mail: dev-
[jira] [Updated] (LUCENE-4132) IndexWriterConfig live settings
[ https://issues.apache.org/jira/browse/LUCENE-4132?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ] Shai Erera updated LUCENE-4132: --- Attachment: LUCENE-4132.patch Patch removes ReadOnlyConfig and the tests from TestIWC that ensure that the same IWC instance isn't shared between IWs. This is no longer needed, since now IW takes IWC and returns LiveConfig, so it cannot be passed to another IW, simply because the compiler won't allow it. This is a better solution IMO than maintaining an AtomicBoolean + tests that enforce that + RuntimeException that is known only during testing, or worse. I don't think we should disable the Builder pattern - our tests use it, and I bet users use it too (my code does). If it bothers anyone, he can separately change all our tests to call the setters one per line. The generics, as Simon said, are just a tool to save code duplication and make sure that IWC and LC have the same getter signatures, and share the live setters. The fact is, no user code will change by that change, and really, no Lucene developer should be affected by it either -- this is just a configuration class, adding set/get methods to it will be as easy as they were before. But now compile-wise, we don't let even expert users change non-live settings. > IndexWriterConfig live settings > --- > > Key: LUCENE-4132 > URL: https://issues.apache.org/jira/browse/LUCENE-4132 > Project: Lucene - Java > Issue Type: Improvement >Reporter: Shai Erera >Priority: Minor > Fix For: 4.0, 5.0 > > Attachments: LUCENE-4132.patch, LUCENE-4132.patch > > > A while ago there was a discussion about making some IW settings "live" and I > remember that RAM buffer size was one of them. Judging from IW code, I see > that RAM buffer can be changed "live" as IW never caches it. > However, I don't remember which other settings were decided to be "live" and > I don't see any documentation in IW nor IWC for that. IW.getConfig mentions: > {code} > * NOTE: some settings may be changed on the > * returned {@link IndexWriterConfig}, and will take > * effect in the current IndexWriter instance. See the > * javadocs for the specific setters in {@link > * IndexWriterConfig} for details. > {code} > But there's no text on e.g. IWC.setRAMBuffer mentioning that. > I think that it'd be good if we make it easier for users to tell which of the > settings are "live" ones. There are few possible ways to do it: > * Introduce a custom @live.setting tag on the relevant IWC.set methods, and > add special text for them in build.xml > ** Or, drop the tag and just document it clearly. > * Separate IWC to two interfaces, LiveConfig and OneTimeConfig (name > proposals are welcome !), have IWC impl both, and introduce another > IW.getLiveConfig which will return that interface, thereby clearly letting > the user know which of the settings are "live". > It'd be good if IWC itself could only expose setXYZ methods for the "live" > settings though. So perhaps, off the top of my head, we can do something like > this: > * Introduce a Config object, which is essentially what IWC is today, and pass > it to IW. > * IW will create a different object, IWC from that Config and IW.getConfig > will return IWC. > * IWC itself will only have setXYZ methods for the "live" settings. > It adds another object, but user code doesn't change - it still creates a > Config object when initializing IW, and need to handle a different type if it > ever calls IW.getConfig. > Maybe that's not such a bad idea? -- This message is automatically generated by JIRA. If you think it was sent incorrectly, please contact your JIRA administrators: https://issues.apache.org/jira/secure/ContactAdministrators!default.jspa For more information on JIRA, see: http://www.atlassian.com/software/jira - To unsubscribe, e-mail: dev-unsubscr...@lucene.apache.org For additional commands, e-mail: dev-h...@lucene.apache.org
[jira] [Updated] (LUCENE-4132) IndexWriterConfig live settings
[ https://issues.apache.org/jira/browse/LUCENE-4132?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ] Shai Erera updated LUCENE-4132: --- Attachment: LUCENE-4132.patch Phew, that was tricky, but here's the end result -- refactored IndexWriterConfig into the following class hierarchy: - ReadOnlyConfig |_ AbstractLiveConfig |_ LiveConfig |_ IndexWriterConfig * IndexWriter now takes ReadOnlyConfig, which is an abstract class with all abstract getters. * LiveConfig is returned from IndexWriter.getConfig(), and is initialized with the ReadOnlyConfig given to IW. It overrides all getters to delegate the call to the given (cloned) config. It is public but with a package-private ctor. * IndexWriterConfig is still the entry object for users to initialize an IndexWriter, and adds its own setters for the non-live settings. * The AbstractLiveConfig in the middle is used for generics and keeping the builder pattern. That way, LiveConfig.set1() and IndexWriterConfig.set1() return the proper type (LiveConfig or IndexWriterConfig respectively). I would have liked IW to keep getting IWC in its ctor, but there's one test that prevents it: TestIndexWriterConfig.testIWCInvalidReuse, which initializes an IW, call getConfig and passes it to another IW (which is invalid). I don't know why it's invalid, as IW clones the given IWC, but that is one reason why I had to factor the getters out to a shared ReadOnlyConfig. ROC is not that bad though -- it kind of protects against IW changing the given config ... At least, no user code should change following these changes, except from changing the variable type used to cache IW.getConfig() to LiveConfig, which is what we want. > IndexWriterConfig live settings > --- > > Key: LUCENE-4132 > URL: https://issues.apache.org/jira/browse/LUCENE-4132 > Project: Lucene - Java > Issue Type: Improvement >Reporter: Shai Erera >Priority: Minor > Fix For: 4.0, 5.0 > > Attachments: LUCENE-4132.patch > > > A while ago there was a discussion about making some IW settings "live" and I > remember that RAM buffer size was one of them. Judging from IW code, I see > that RAM buffer can be changed "live" as IW never caches it. > However, I don't remember which other settings were decided to be "live" and > I don't see any documentation in IW nor IWC for that. IW.getConfig mentions: > {code} > * NOTE: some settings may be changed on the > * returned {@link IndexWriterConfig}, and will take > * effect in the current IndexWriter instance. See the > * javadocs for the specific setters in {@link > * IndexWriterConfig} for details. > {code} > But there's no text on e.g. IWC.setRAMBuffer mentioning that. > I think that it'd be good if we make it easier for users to tell which of the > settings are "live" ones. There are few possible ways to do it: > * Introduce a custom @live.setting tag on the relevant IWC.set methods, and > add special text for them in build.xml > ** Or, drop the tag and just document it clearly. > * Separate IWC to two interfaces, LiveConfig and OneTimeConfig (name > proposals are welcome !), have IWC impl both, and introduce another > IW.getLiveConfig which will return that interface, thereby clearly letting > the user know which of the settings are "live". > It'd be good if IWC itself could only expose setXYZ methods for the "live" > settings though. So perhaps, off the top of my head, we can do something like > this: > * Introduce a Config object, which is essentially what IWC is today, and pass > it to IW. > * IW will create a different object, IWC from that Config and IW.getConfig > will return IWC. > * IWC itself will only have setXYZ methods for the "live" settings. > It adds another object, but user code doesn't change - it still creates a > Config object when initializing IW, and need to handle a different type if it > ever calls IW.getConfig. > Maybe that's not such a bad idea? -- This message is automatically generated by JIRA. If you think it was sent incorrectly, please contact your JIRA administrators: https://issues.apache.org/jira/secure/ContactAdministrators!default.jspa For more information on JIRA, see: http://www.atlassian.com/software/jira - To unsubscribe, e-mail: dev-unsubscr...@lucene.apache.org For additional commands, e-mail: dev-h...@lucene.apache.org