Details

    • Type: Task Task
    • Status: Closed
    • Priority: Blocker Blocker
    • Resolution: Fixed
    • Affects Version/s: None
    • Fix Version/s: 5.0
    • Component/s: None
    • Labels:
      None

      Description

      On the Solr Cores and solr.xml page, the Solr Reference Guide says:

      Starting in Solr 4.3, Solr will maintain two distinct formats for solr.xml, the legacy and discovery modes. The former is the format we have become accustomed to in which all of the cores one wishes to define in a Solr instance are defined in solr.xml in <cores><core/>...<core/></cores> tags. This format will continue to be supported through the entire 4.x code line.

      As of Solr 5.0 this form of solr.xml will no longer be supported. Instead Solr will support core discovery. [...]

      The new "core discovery mode" structure for solr.xml will become mandatory as of Solr 5.0, see: Format of solr.xml.

      AFAICT, nothing has been done to remove legacy solr.xml mode from 5.0 or trunk.

      1. SOLR-6840.patch
        279 kB
        Alan Woodward
      2. SOLR-6840.patch
        3.40 MB
        Alan Woodward
      3. SOLR-6840.patch
        470 kB
        Alan Woodward
      4. SOLR-6840.patch
        323 kB
        Alan Woodward
      5. SOLR-6840.patch
        174 kB
        Alan Woodward
      6. SOLR-6840.patch
        91 kB
        Erick Erickson

        Activity

        Hide
        Steve Rowe added a comment -

        Erick Erickson, I'm hoping you'll have time to work on this.

        Show
        Steve Rowe added a comment - Erick Erickson , I'm hoping you'll have time to work on this.
        Hide
        Erick Erickson added a comment -

        Gahhhh, Of course I don't have time to work on this. And how is that different than any other time

        You're right, though, this needs to be addressed before the 5.0 release. So I'll mark it "blocker", assign it to myself and see if I can get to it in the next week.

        Alan Woodward if you have any cycles please don't hesitate to grab it, you won't offend me at all....

        Show
        Erick Erickson added a comment - Gahhhh, Of course I don't have time to work on this. And how is that different than any other time You're right, though, this needs to be addressed before the 5.0 release. So I'll mark it "blocker", assign it to myself and see if I can get to it in the next week. Alan Woodward if you have any cycles please don't hesitate to grab it, you won't offend me at all....
        Hide
        Erick Erickson added a comment -

        OK, the bare beginnings of the patch. What it does:

        1> removes all of the references I could find to any <cores.... tags.

        2> Tries to deal with SolrTestCaseJ4 and TestHarness. TestHarness used to have a hard-coded <cores bit, but it doesn't now. Basically I create a core.properties file in the test area and include a name, schema and config, now the initCore tests in SolrTestCaseJ4 set the env var to the file names being passed in. There are comments particularly in SolrTestDase4J that show an alternative approach, but that copies files all over the place and I really don't like that at all. I'm utterly sure that there's more work to do with these two files.....

        3> removes anything like solr*old*.xml from the source tree.

        Gets 78 more tests to pass. There are only 158 failing now. It started with 236 failing. So it's at least progress. Sounds worse than it is I suspect.

        So I'm putting it up for comments on the approach, particularly the new core.properties, SolrTestCaseJ4 and TestHarness (not, I really haven't worked on this last).

        And I'm also wondering if there's any good way to split this work up. There's going to be a LOT of tedious work to get all the tests to pass. But reconciling multiple people's patches will be an absolute pain. Maybe if people work on bits just put up patches (appropriately labeled) that just include the delta (or as close as one can get easily) and I'll reconcile and keep the uber-patch as current as I can? I may be able to look at TestHarness a bit more tonight...

        Show
        Erick Erickson added a comment - OK, the bare beginnings of the patch. What it does: 1> removes all of the references I could find to any <cores.... tags. 2> Tries to deal with SolrTestCaseJ4 and TestHarness. TestHarness used to have a hard-coded <cores bit, but it doesn't now. Basically I create a core.properties file in the test area and include a name, schema and config, now the initCore tests in SolrTestCaseJ4 set the env var to the file names being passed in. There are comments particularly in SolrTestDase4J that show an alternative approach, but that copies files all over the place and I really don't like that at all. I'm utterly sure that there's more work to do with these two files..... 3> removes anything like solr*old*.xml from the source tree. Gets 78 more tests to pass. There are only 158 failing now. It started with 236 failing. So it's at least progress. Sounds worse than it is I suspect. So I'm putting it up for comments on the approach, particularly the new core.properties, SolrTestCaseJ4 and TestHarness (not, I really haven't worked on this last). And I'm also wondering if there's any good way to split this work up. There's going to be a LOT of tedious work to get all the tests to pass. But reconciling multiple people's patches will be an absolute pain. Maybe if people work on bits just put up patches (appropriately labeled) that just include the delta (or as close as one can get easily) and I'll reconcile and keep the uber-patch as current as I can? I may be able to look at TestHarness a bit more tonight...
        Hide
        Erick Erickson added a comment -

        OK, we need to decide how we'll approach persistence in particular.

        1> There's no persist=false in the new solr.xml, ConfigSolrXml.isPersistent always returns true. Or I'm missing something again. 75 tests fail since they try to write out to the solr test files tree and the framework (rightly) barfs (see <2> below). This assumes that I do the trick in <2> of my previous comment. Any suggestions?

        2> You can get around <1> by copying all the config files to a temp dir and making solr_home point there. For many, many tests. This means that a zillion files get copied all over the place. This fixes 75 test failures.

        So, in general what's the story with dealing with persistence in the new format, particularly in the test world? I really don't want to re-introduce the whole horrible persistence stuff.... Alan Woodward I'd be particularly interested in your take. Is there any real reason for ConfigSolrXml.isPersistent to return true?

        And also note that there are a bunch of places that really need some attention. I blindly removed all the <cores... tags. There's more than one place where that means the in-test solr.xml file (as a string) looks like <solr></solr>, which is useless and ought to be dealt with one-by-one.

        Show
        Erick Erickson added a comment - OK, we need to decide how we'll approach persistence in particular. 1> There's no persist=false in the new solr.xml, ConfigSolrXml.isPersistent always returns true. Or I'm missing something again. 75 tests fail since they try to write out to the solr test files tree and the framework (rightly) barfs (see <2> below). This assumes that I do the trick in <2> of my previous comment. Any suggestions? 2> You can get around <1> by copying all the config files to a temp dir and making solr_home point there. For many, many tests. This means that a zillion files get copied all over the place. This fixes 75 test failures. So, in general what's the story with dealing with persistence in the new format, particularly in the test world? I really don't want to re-introduce the whole horrible persistence stuff.... Alan Woodward I'd be particularly interested in your take. Is there any real reason for ConfigSolrXml.isPersistent to return true? And also note that there are a bunch of places that really need some attention. I blindly removed all the <cores... tags. There's more than one place where that means the in-test solr.xml file (as a string) looks like <solr></solr>, which is useless and ought to be dealt with one-by-one.
        Hide
        Alan Woodward added a comment -

        I don't think you can just remove <cores> entries, as there were a whole bunch of other attributes specified on it in addition to listing the cores.

        Persistence makes no sense with the new setup, as the information in ConfigSolr is immutable for the lifetime of the container. So really isPersistent() should just be removed.

        Ideally you shouldn't have to write properties files anywhere for tests (unless you're explicitly testing the core discovery logic). TestHarness and/or SolrTestCaseJ4 should have their own CoresLocator implementation that returns a CoreDescriptor with the appropriate schema and config settings. The whole point of the CoresLocator abstraction is that you're not tied to any particular file format for testing.

        Show
        Alan Woodward added a comment - I don't think you can just remove <cores> entries, as there were a whole bunch of other attributes specified on it in addition to listing the cores. Persistence makes no sense with the new setup, as the information in ConfigSolr is immutable for the lifetime of the container. So really isPersistent() should just be removed. Ideally you shouldn't have to write properties files anywhere for tests (unless you're explicitly testing the core discovery logic). TestHarness and/or SolrTestCaseJ4 should have their own CoresLocator implementation that returns a CoreDescriptor with the appropriate schema and config settings. The whole point of the CoresLocator abstraction is that you're not tied to any particular file format for testing.
        Hide
        Erick Erickson added a comment -

        Alan:

        Right, thanks. Before I went off the deep end I wanted to be sure of the intent.

        bq: I don't think you can just remove <cores> entries, as there were a whole bunch of other attributes specified on it in addition to listing the cores.

        Of course I can't blindly remove all <cores... entries and expect it to "just work". But by doing so I've unambiguously found all of the places where we need to do something to get tests to pass though

        bq:.... isPersistent() should just be removed.

        Great, that was the big question for me, I'll give that a whirl tonight. Which will have the consequence of finding/modifying anything that uses it. Which should prevent these things from being written in the future. Which should cause all the tests that pass with copying junk around to pass.... And on to the next failing tests...

        bq: Ideally you shouldn't have to write properties files anywhere for tests
        Agreed, and with the env var substitution trick I don't have to, just have to work out the isPersistent bit.

        Anyway, thanks. The crucial bit was your statement that "Persistence makes no sense". And glad I am that it's gone, getting it right was a major pain. I'm not entirely sure that continues to work on the individual core.properties files, but theoretically anything that was changing them had to create a tmp directory first someplace b/c the test framework wouldn't let them write to the source tree.

        Anyway, probably a day or two before I can get any more done...

        Show
        Erick Erickson added a comment - Alan: Right, thanks. Before I went off the deep end I wanted to be sure of the intent. bq: I don't think you can just remove <cores> entries, as there were a whole bunch of other attributes specified on it in addition to listing the cores. Of course I can't blindly remove all <cores... entries and expect it to "just work". But by doing so I've unambiguously found all of the places where we need to do something to get tests to pass though bq:.... isPersistent() should just be removed. Great, that was the big question for me, I'll give that a whirl tonight. Which will have the consequence of finding/modifying anything that uses it. Which should prevent these things from being written in the future. Which should cause all the tests that pass with copying junk around to pass.... And on to the next failing tests... bq: Ideally you shouldn't have to write properties files anywhere for tests Agreed, and with the env var substitution trick I don't have to, just have to work out the isPersistent bit. Anyway, thanks. The crucial bit was your statement that "Persistence makes no sense". And glad I am that it's gone, getting it right was a major pain. I'm not entirely sure that continues to work on the individual core.properties files, but theoretically anything that was changing them had to create a tmp directory first someplace b/c the test framework wouldn't let them write to the source tree. Anyway, probably a day or two before I can get any more done...
        Hide
        Alan Woodward added a comment -

        I've tried out an alternative approach, by just removing the ConfigSolrXmlOld code and everything depending on it, and then seeing what fails. Here's a checkpoint patch. So far all the tests in org.apache.solr.core are passing, but all of the distributed tests currently fail because the default solr.xml created a 'collection1' core for them. Trying to work out how to get them passing now.

        Show
        Alan Woodward added a comment - I've tried out an alternative approach, by just removing the ConfigSolrXmlOld code and everything depending on it, and then seeing what fails. Here's a checkpoint patch. So far all the tests in org.apache.solr.core are passing, but all of the distributed tests currently fail because the default solr.xml created a 'collection1' core for them. Trying to work out how to get them passing now.
        Hide
        Erick Erickson added a comment -

        You're doing yeoman's duty on this, a mere 174K patch... so far

        I might recommend for this phase, leaving the failIfFound tests in ConfigSolrXml for a little on the theory that there are all sorts of nooks and crannies that exist. Probably take them out when before checking things in, but between now and then it'd be useful to fail there I think. There are still a lot of solr.xml files out there after the patch that have <cores... It's actually an open question for me whether they are useful, maybe they should just be deleted wholesale...

        If you get to a point where you want to take a break, I can take a whack at working on some of the test cases working.

        Show
        Erick Erickson added a comment - You're doing yeoman's duty on this, a mere 174K patch... so far I might recommend for this phase, leaving the failIfFound tests in ConfigSolrXml for a little on the theory that there are all sorts of nooks and crannies that exist. Probably take them out when before checking things in, but between now and then it'd be useful to fail there I think. There are still a lot of solr.xml files out there after the patch that have <cores... It's actually an open question for me whether they are useful, maybe they should just be deleted wholesale... If you get to a point where you want to take a break, I can take a whack at working on some of the test cases working.
        Hide
        Alan Woodward added a comment -

        I'm working on the distributed tests now. Rather than having a core predefined in solr.xml, I'm creating them via the core admin API in setup, but I've bumped up against a bit of an API mismatch here. If you want to create cores on an empty node via SolrJ, you create a SolrServer pointing at the /solr context, but if you then want to query a specific core you need to create a second SolrServer that points at that core. In SolrCloud you can set the 'collection' parameter to get round this, but in standard multicore Solr that doesn't work.

        Show
        Alan Woodward added a comment - I'm working on the distributed tests now. Rather than having a core predefined in solr.xml, I'm creating them via the core admin API in setup, but I've bumped up against a bit of an API mismatch here. If you want to create cores on an empty node via SolrJ, you create a SolrServer pointing at the /solr context, but if you then want to query a specific core you need to create a second SolrServer that points at that core. In SolrCloud you can set the 'collection' parameter to get round this, but in standard multicore Solr that doesn't work.
        Hide
        Alexandre Rafalovitch added a comment -

        Probably not caught by the tests, but the DIH example uses the legacy form of solr.xml. Somebody need to add empty core.properties files, etc. The only issue is that one of the cores was declared as default, not sure if any examples rely on this.

        Show
        Alexandre Rafalovitch added a comment - Probably not caught by the tests, but the DIH example uses the legacy form of solr.xml. Somebody need to add empty core.properties files, etc. The only issue is that one of the cores was declared as default, not sure if any examples rely on this.
        Hide
        Erick Erickson added a comment -

        Alan Woodward I've actually got a pretty open week ahead, I can move this forward for a while if the patch is your most current....

        Or not, up to you.

        Alexandre Rafalovitch Yeah, one of the tasks I have on my list is to grep for <cores in all the files in the project to be sure they're all gone. Although the parsing code should have errors if any are still in there.... We'll see.

        Show
        Erick Erickson added a comment - Alan Woodward I've actually got a pretty open week ahead, I can move this forward for a while if the patch is your most current.... Or not, up to you. Alexandre Rafalovitch Yeah, one of the tasks I have on my list is to grep for <cores in all the files in the project to be sure they're all gone. Although the parsing code should have errors if any are still in there.... We'll see.
        Hide
        Alan Woodward added a comment -

        Hey, sorry have been away from internet, and then ill.

        I've moved on a bit from the patch I posted above. Removing the default core from solr.xml has uncovered a few other API corners that I'd like to smooth over (SOLR-6894 being the first of these). I'll try and get up to date in the next couple of days, and then maybe we can open a branch and divide the work up a bit?

        What I really want to do is make this work programmatically (ie, the test cases create cores using the CoreAdmin or CollectionAdmin API) rather than just copying files all over the place. Otherwise you end up like we are now, with almost all distributed tests relying on the test classes knowing the implementation details of how cores or collections are set up, meaning that they all have to be changed when those implementation details change.

        Show
        Alan Woodward added a comment - Hey, sorry have been away from internet, and then ill. I've moved on a bit from the patch I posted above. Removing the default core from solr.xml has uncovered a few other API corners that I'd like to smooth over ( SOLR-6894 being the first of these). I'll try and get up to date in the next couple of days, and then maybe we can open a branch and divide the work up a bit? What I really want to do is make this work programmatically (ie, the test cases create cores using the CoreAdmin or CollectionAdmin API) rather than just copying files all over the place. Otherwise you end up like we are now, with almost all distributed tests relying on the test classes knowing the implementation details of how cores or collections are set up, meaning that they all have to be changed when those implementation details change.
        Hide
        Erick Erickson added a comment -

        Well, it was Christmas! But hey! There's almost a week between now and really getting back to work, get cracking! .

        NP, I happily went off in to the weeds on SOLR-6888 anyway.

        bq: with almost all distributed tests relying on the test classes knowing the implementation details of how cores or collections are set up

        Huge +1. It's really annoying to try to untangle all of that. Not to mention the number of places that have their own solr.xml as a string in the test case.....

        bq: then maybe we can open a branch and divide the work up a bit

        Works for me. And anyone else who wants to chime in......

        Show
        Erick Erickson added a comment - Well, it was Christmas! But hey! There's almost a week between now and really getting back to work, get cracking! . NP, I happily went off in to the weeds on SOLR-6888 anyway. bq: with almost all distributed tests relying on the test classes knowing the implementation details of how cores or collections are set up Huge +1. It's really annoying to try to untangle all of that. Not to mention the number of places that have their own solr.xml as a string in the test case..... bq: then maybe we can open a branch and divide the work up a bit Works for me. And anyone else who wants to chime in......
        Hide
        Alexandre Rafalovitch added a comment -

        If there is a brunch, I am happy to - at least - cleanup DIH example. I suspect it would be fairly trivial.

        But there is a question of whether to replace solr.xml with minimal version (<solr/>) or with explicit defaults like the other examples do. Since it is a public version, that's worth a thought.

        Show
        Alexandre Rafalovitch added a comment - If there is a brunch, I am happy to - at least - cleanup DIH example. I suspect it would be fairly trivial. But there is a question of whether to replace solr.xml with minimal version (<solr/>) or with explicit defaults like the other examples do. Since it is a public version, that's worth a thought.
        Hide
        Erik Hatcher added a comment -

        The example/example-DIH/solr still has a solr.xml - so that tree will need to be converted over too, FYI

        Show
        Erik Hatcher added a comment - The example/example-DIH/solr still has a solr.xml - so that tree will need to be converted over too, FYI
        Hide
        Erik Hatcher added a comment -

        The example/example-DIH/solr still has a solr.xml ...

        And now that I look deeper, I see there are many solr.xml's out there and I imagine that would be part of this effort is to convert all those over. Sorry for the noise of the obvious.

        Show
        Erik Hatcher added a comment - The example/example-DIH/solr still has a solr.xml ... And now that I look deeper, I see there are many solr.xml's out there and I imagine that would be part of this effort is to convert all those over. Sorry for the noise of the obvious.
        Hide
        Alan Woodward added a comment -

        Latest patch - there are still a couple of test failures, mainly to do with Cloud setups. I'm working on those now.

        Show
        Alan Woodward added a comment - Latest patch - there are still a couple of test failures, mainly to do with Cloud setups. I'm working on those now.
        Hide
        Alan Woodward added a comment -

        So the problem with Cloud setups appears to be due to the control jetty. Previously, the cores for all the jetties were being predefined in solr.xml, so the cluster came up automatically with cores in place. However, now I'm creating the test collections with a CollectionAdmin request, and this means that cores sometimes get created on the control jetty, as it's part of the cluster. This then causes failures further down the line.

        So my question really is, what's the purpose of the control jetty here? In non-cloud distributed tests, it's there to check that distributed results are the same as results from a single core. In cloud tests, we have the singly-sharded control collection for this. Should I be setting it up so that the control Jetty doesn't join the cluster at all?

        Show
        Alan Woodward added a comment - So the problem with Cloud setups appears to be due to the control jetty. Previously, the cores for all the jetties were being predefined in solr.xml, so the cluster came up automatically with cores in place. However, now I'm creating the test collections with a CollectionAdmin request, and this means that cores sometimes get created on the control jetty, as it's part of the cluster. This then causes failures further down the line. So my question really is, what's the purpose of the control jetty here? In non-cloud distributed tests, it's there to check that distributed results are the same as results from a single core. In cloud tests, we have the singly-sharded control collection for this. Should I be setting it up so that the control Jetty doesn't join the cluster at all?
        Hide
        Hoss Man added a comment -

        Should I be setting it up so that the control Jetty doesn't join the cluster at all?

        my understanding for the longest time was that the control jetty/collection1 was suppose to exist for exactly this purpose – to be a "standalone single core" equivilent of the distributed "collection1" for the purpose of comparisons.

        but then in SOLR-2894 and SOLR-6379 miller made comments about how the control jetty was suppose to work in cloud based test that confused me and still confuse me and i defer you to those comments rather then trying to explain them.

        Mark Miller: can you help clarify things for Alan so he can get these tests working w/o the legacy solr.xml support in there?

        Show
        Hoss Man added a comment - Should I be setting it up so that the control Jetty doesn't join the cluster at all? my understanding for the longest time was that the control jetty/collection1 was suppose to exist for exactly this purpose – to be a "standalone single core" equivilent of the distributed "collection1" for the purpose of comparisons. but then in SOLR-2894 and SOLR-6379 miller made comments about how the control jetty was suppose to work in cloud based test that confused me and still confuse me and i defer you to those comments rather then trying to explain them. Mark Miller : can you help clarify things for Alan so he can get these tests working w/o the legacy solr.xml support in there?
        Hide
        Mark Miller added a comment -

        We would prefer the control to be a single replica, single shard in it's own SolrCloud cluster, eg it's own ZooKeeper chroot, not part of the test cluster.

        Due to some history and timing and ease and complications, etc, there was some bleed over. It's probably best to tackles unbleeding that here.

        Show
        Mark Miller added a comment - We would prefer the control to be a single replica, single shard in it's own SolrCloud cluster, eg it's own ZooKeeper chroot, not part of the test cluster. Due to some history and timing and ease and complications, etc, there was some bleed over. It's probably best to tackles unbleeding that here.
        Hide
        Alan Woodward added a comment -

        I'm going to try using MiniSolrCloudCluster to launch both the control cluster and test clusters, and see if that helps any. At the moment this is all very difficult to follow and untangle...

        Show
        Alan Woodward added a comment - I'm going to try using MiniSolrCloudCluster to launch both the control cluster and test clusters, and see if that helps any. At the moment this is all very difficult to follow and untangle...
        Hide
        Mark Miller added a comment -

        I'm going to try using MiniSolrCloudCluster to launch both the control cluster and test clusters

        We would like to end up there anyway, so it wouldn't be wasted effort. Same as the control issue above, we don't handle all of this by class extension because it's ideal, it's mainly because of compromises made when choosing which Solr code to leverage when starting SolrCloud coding.

        I talked about this change some with Greg when he was writing the MiniSolrCloudCluster, but neither of us had the time to unravel this at the time. As test stability and other things have improved though, hopefully this has only gotten easier to do.

        Show
        Mark Miller added a comment - I'm going to try using MiniSolrCloudCluster to launch both the control cluster and test clusters We would like to end up there anyway, so it wouldn't be wasted effort. Same as the control issue above, we don't handle all of this by class extension because it's ideal, it's mainly because of compromises made when choosing which Solr code to leverage when starting SolrCloud coding. I talked about this change some with Greg when he was writing the MiniSolrCloudCluster, but neither of us had the time to unravel this at the time. As test stability and other things have improved though, hopefully this has only gotten easier to do.
        Hide
        Erick Erickson added a comment -

        Hmmm, not sure how all this plays with SOLR-6902, Ramkumar was thinking about restructuring some of the inheritance, is his work there complimentary? Unnecessary? Soon to be obsolete? No opinion?

        Show
        Erick Erickson added a comment - Hmmm, not sure how all this plays with SOLR-6902 , Ramkumar was thinking about restructuring some of the inheritance, is his work there complimentary? Unnecessary? Soon to be obsolete? No opinion?
        Hide
        Alan Woodward added a comment -

        I'm only changing the createServers() method at the moment, which I think fits with Ram's patch, so feel free to commit that one.

        Show
        Alan Woodward added a comment - I'm only changing the createServers() method at the moment, which I think fits with Ram's patch, so feel free to commit that one.
        Hide
        Alan Woodward added a comment -

        Latest patch. Am working through the cloud tests still, which is taking a while, particularly now that the control cluster is entirely separate.

        I'm getting odd fails from somewhere inside HttpClient occasionally, saying "Scheme 'http' not registered", which I guess is something to do with SSL? Will chase that down too.

        Show
        Alan Woodward added a comment - Latest patch. Am working through the cloud tests still, which is taking a while, particularly now that the control cluster is entirely separate. I'm getting odd fails from somewhere inside HttpClient occasionally, saying "Scheme 'http' not registered", which I guess is something to do with SSL? Will chase that down too.
        Hide
        Mark Miller added a comment -

        bq saying "Scheme 'http' not registered",

        Perhaps something assuming http for an address when it needs to use a call to get http or https.

        Show
        Mark Miller added a comment - bq saying "Scheme 'http' not registered", Perhaps something assuming http for an address when it needs to use a call to get http or https.
        Hide
        Alan Woodward added a comment -

        Yeah, I was being too dumb in creating SolrClients. I've added a .getSolrClient(HttpClient) method to JettySolrRunner which deals with this correctly.

        Show
        Alan Woodward added a comment - Yeah, I was being too dumb in creating SolrClients. I've added a .getSolrClient(HttpClient) method to JettySolrRunner which deals with this correctly.
        Hide
        Steve Davids added a comment -

        Something doesn't seem right here, there is a static http client builder method that will generate an HttpClient instance in the correct state based on the system properties of the current actively running test. You shouldn't need to specify your own instance of HttpClient to build a Solr Client. I can take a look at this later tonight if you want, was there a particular test failure that I should hone in on?

        Show
        Steve Davids added a comment - Something doesn't seem right here, there is a static http client builder method that will generate an HttpClient instance in the correct state based on the system properties of the current actively running test. You shouldn't need to specify your own instance of HttpClient to build a Solr Client. I can take a look at this later tonight if you want, was there a particular test failure that I should hone in on?
        Hide
        Alan Woodward added a comment -

        Right, there's a single HttpClient (built using that static method) that the test then shares between all its SolrClients. The problem wasn't in the HttpClient itself, it was in how the SolrClients were being built (specifically, how URLs for HttpSolrClient objects were constructed).

        Show
        Alan Woodward added a comment - Right, there's a single HttpClient (built using that static method) that the test then shares between all its SolrClients. The problem wasn't in the HttpClient itself, it was in how the SolrClients were being built (specifically, how URLs for HttpSolrClient objects were constructed).
        Hide
        Alan Woodward added a comment -

        I'm going to have abandon the idea of using MiniSolrCloudCluster for now, I think. There are so many tests that make assumptions about which cores have been placed where that trying to use the CoreAdmin and CollectionAdmin APIs break too many things. And there's still a nasty API mismatch on SolrClient which means that you need to use a different client to send admin requests and collection requests, which makes keeping track of which clients are being used for what a nightmare.

        In order to get this in and tests running, I'm going to hack it up so that it creates core.properties files in the relevant directories. Once that's done we can look at moving everything over to using MiniSolrCloudCluster, and working on a better SolrClient API, but that will have to be done one test at a time, and I don't think we should hold up the 5.0 release for the sake of the test framework.

        Show
        Alan Woodward added a comment - I'm going to have abandon the idea of using MiniSolrCloudCluster for now, I think. There are so many tests that make assumptions about which cores have been placed where that trying to use the CoreAdmin and CollectionAdmin APIs break too many things. And there's still a nasty API mismatch on SolrClient which means that you need to use a different client to send admin requests and collection requests, which makes keeping track of which clients are being used for what a nightmare. In order to get this in and tests running, I'm going to hack it up so that it creates core.properties files in the relevant directories. Once that's done we can look at moving everything over to using MiniSolrCloudCluster, and working on a better SolrClient API, but that will have to be done one test at a time, and I don't think we should hold up the 5.0 release for the sake of the test framework.
        Hide
        Erick Erickson added a comment -

        Alan:

        Note that you may not really need to put anything in core.properties. As long as the file just exists it's sufficient if you're OK with all the values being defaults. E.g. the name of the directory it's found in becomes the name of the core etc., at least in the non-cloud case. In cloud mode it probably needs more though.

        FWIW

        Show
        Erick Erickson added a comment - Alan: Note that you may not really need to put anything in core.properties. As long as the file just exists it's sufficient if you're OK with all the values being defaults. E.g. the name of the directory it's found in becomes the name of the core etc., at least in the non-cloud case. In cloud mode it probably needs more though. FWIW
        Hide
        Alan Woodward added a comment -

        Patch for trunk - am running precommit now, and will commit to trunk once that's done.

        5x will require a slightly amended patch, as the morphline tests don't run under Java 8 so I haven't been able to check those. My plan is to let Jenkins work on this today, and then backport to 5.x and 5.0 tomorrow.

        Show
        Alan Woodward added a comment - Patch for trunk - am running precommit now, and will commit to trunk once that's done. 5x will require a slightly amended patch, as the morphline tests don't run under Java 8 so I haven't been able to check those. My plan is to let Jenkins work on this today, and then backport to 5.x and 5.0 tomorrow.
        Hide
        Alan Woodward added a comment -

        Oops, wrong patch. Here's one against today's trunk...

        Show
        Alan Woodward added a comment - Oops, wrong patch. Here's one against today's trunk...
        Hide
        Uwe Schindler added a comment -

        5x will require a slightly amended patch, as the morphline tests don't run under Java 8 so I haven't been able to check those

        I think SAXON was already updated! So I see no reason why the assumes are still there. Can you try to comment out the Java 8 assumes? They were already removed in morphlines-sell, but not in core. Strange:

        Revision: 1642014
        Author: markrmiller
        Date: Donnerstag, 27. November 2014 04:05:17
        Message:
        SOLR-6799: Update Saxon-HE to 9.6.0-2.


        Modified : /lucene/dev/trunk/lucene/ivy-versions.properties
        Modified : /lucene/dev/trunk/solr/CHANGES.txt
        Modified : /lucene/dev/trunk/solr/contrib/morphlines-cell/src/test/org/apache/solr/morphlines/cell/SolrCellMorphlineTest.java
        Added : /lucene/dev/trunk/solr/licenses/Saxon-HE-9.6.0-2.jar.sha1

        This one does not change the other tests. Maybe that was an oversight.

        Show
        Uwe Schindler added a comment - 5x will require a slightly amended patch, as the morphline tests don't run under Java 8 so I haven't been able to check those I think SAXON was already updated! So I see no reason why the assumes are still there. Can you try to comment out the Java 8 assumes? They were already removed in morphlines-sell, but not in core. Strange: Revision: 1642014 Author: markrmiller Date: Donnerstag, 27. November 2014 04:05:17 Message: SOLR-6799 : Update Saxon-HE to 9.6.0-2. Modified : /lucene/dev/trunk/lucene/ivy-versions.properties Modified : /lucene/dev/trunk/solr/CHANGES.txt Modified : /lucene/dev/trunk/solr/contrib/morphlines-cell/src/test/org/apache/solr/morphlines/cell/SolrCellMorphlineTest.java Added : /lucene/dev/trunk/solr/licenses/Saxon-HE-9.6.0-2.jar.sha1 This one does not change the other tests. Maybe that was an oversight.
        Hide
        Uwe Schindler added a comment - - edited

        I reopened SOLR-6799 and will commit the removal of the assume once I verified that it works with Java 8.

        Show
        Uwe Schindler added a comment - - edited I reopened SOLR-6799 and will commit the removal of the assume once I verified that it works with Java 8.
        Hide
        Uwe Schindler added a comment -

        Alan Woodward: I commited the removal of the Java 8 assumes! So you should be able to test the morphlines/mapreduce-tests with your patch!

        Show
        Uwe Schindler added a comment - Alan Woodward : I commited the removal of the Java 8 assumes! So you should be able to test the morphlines/mapreduce-tests with your patch!
        Hide
        Alan Woodward added a comment -

        And they all fail! Some more work to do here then...

        Thanks Uwe.

        Show
        Alan Woodward added a comment - And they all fail! Some more work to do here then... Thanks Uwe.
        Hide
        Uwe Schindler added a comment -

        As expected by you. I hope you get it running!
        The test setup of those is strange, because they reuse the test-resources from other modules... Sorry, I tried to clean that up -> impossible

        Show
        Uwe Schindler added a comment - As expected by you. I hope you get it running! The test setup of those is strange, because they reuse the test-resources from other modules... Sorry, I tried to clean that up -> impossible
        Hide
        ASF subversion and git services added a comment -

        Commit 1652995 from Alan Woodward in branch 'dev/trunk'
        [ https://svn.apache.org/r1652995 ]

        SOLR-6840: Remove support for old-style solr.xml

        Show
        ASF subversion and git services added a comment - Commit 1652995 from Alan Woodward in branch 'dev/trunk' [ https://svn.apache.org/r1652995 ] SOLR-6840 : Remove support for old-style solr.xml
        Hide
        Alan Woodward added a comment -

        I'm going to let this break^H^H^H^H^H bake in trunk for 24 hours or so and see what it throws up before backporting.

        Show
        Alan Woodward added a comment - I'm going to let this break^H^H^H^H^H bake in trunk for 24 hours or so and see what it throws up before backporting.
        Hide
        Alan Woodward added a comment -

        One extra thing: a side effect of removing the old-style solr.xml is that there's no longer support for a default core (a lot of the changes here are to SolrClients that need to explicitly ask for their collection now). Is it worth having a separate JIRA issue just for that? I'll add a note to the CHANGES and MIGRATE files in SOLR-6976 as well.

        Show
        Alan Woodward added a comment - One extra thing: a side effect of removing the old-style solr.xml is that there's no longer support for a default core (a lot of the changes here are to SolrClients that need to explicitly ask for their collection now). Is it worth having a separate JIRA issue just for that? I'll add a note to the CHANGES and MIGRATE files in SOLR-6976 as well.
        Hide
        ASF subversion and git services added a comment -

        Commit 1653073 from Alan Woodward in branch 'dev/trunk'
        [ https://svn.apache.org/r1653073 ]

        SOLR-6840: Fix multicore example tests

        Show
        ASF subversion and git services added a comment - Commit 1653073 from Alan Woodward in branch 'dev/trunk' [ https://svn.apache.org/r1653073 ] SOLR-6840 : Fix multicore example tests
        Hide
        ASF subversion and git services added a comment -

        Commit 1653212 from Alan Woodward in branch 'dev/trunk'
        [ https://svn.apache.org/r1653212 ]

        SOLR-6840: Fix clustering test

        Show
        ASF subversion and git services added a comment - Commit 1653212 from Alan Woodward in branch 'dev/trunk' [ https://svn.apache.org/r1653212 ] SOLR-6840 : Fix clustering test
        Hide
        ASF subversion and git services added a comment -

        Commit 1653332 from Alan Woodward in branch 'dev/branches/branch_5x'
        [ https://svn.apache.org/r1653332 ]

        SOLR-6840: Remove support for old-style solr.xml

        Show
        ASF subversion and git services added a comment - Commit 1653332 from Alan Woodward in branch 'dev/branches/branch_5x' [ https://svn.apache.org/r1653332 ] SOLR-6840 : Remove support for old-style solr.xml
        Hide
        ASF subversion and git services added a comment -

        Commit 1653339 from Alan Woodward in branch 'dev/branches/lucene_solr_5_0'
        [ https://svn.apache.org/r1653339 ]

        SOLR-6840: Remove support for old-style solr.xml

        Show
        ASF subversion and git services added a comment - Commit 1653339 from Alan Woodward in branch 'dev/branches/lucene_solr_5_0' [ https://svn.apache.org/r1653339 ] SOLR-6840 : Remove support for old-style solr.xml
        Hide
        Alan Woodward added a comment -

        Phew. Thanks everyone.

        Show
        Alan Woodward added a comment - Phew. Thanks everyone.
        Hide
        Anshum Gupta added a comment -

        Bulk close after 5.0 release.

        Show
        Anshum Gupta added a comment - Bulk close after 5.0 release.

          People

          • Assignee:
            Alan Woodward
            Reporter:
            Steve Rowe
          • Votes:
            0 Vote for this issue
            Watchers:
            8 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved:

              Development