Details

    • Type: New Feature New Feature
    • Status: Closed
    • Priority: Major Major
    • Resolution: Fixed
    • Affects Version/s: None
    • Fix Version/s: 4.0, 5.0
    • Component/s: SolrCloud
    • Labels:
      None

      Description

      When creating a new collection, a placeholder for each shard should be created.

      1. SOLR-3088.patch
        11 kB
        Sami Siren
      2. SOLR-3088.patch
        10 kB
        Sami Siren

        Issue Links

          Activity

          Hide
          Yonik Seeley added a comment -

          There are a couple of reasons...

          • the cluster should be self-describing so that one can start up a node and say "go join this cluster" and need not know anything else (like the number of shards that is currently passed when starting each node)
          • right now, one can bring up one shard of a 3 shard cluster and then successfully index a bunch of docs (oops, they all went into shard1)

          I imagine the placeholders should be both in the ZK structure under /collections and in clusterstate

          Show
          Yonik Seeley added a comment - There are a couple of reasons... the cluster should be self-describing so that one can start up a node and say "go join this cluster" and need not know anything else (like the number of shards that is currently passed when starting each node) right now, one can bring up one shard of a 3 shard cluster and then successfully index a bunch of docs (oops, they all went into shard1) I imagine the placeholders should be both in the ZK structure under /collections and in clusterstate
          Hide
          Sami Siren added a comment -

          the cluster should be self-describing so that one can start up a node and say "go join this cluster"

          Do you mean that the new node would automatically discover from the placeholders how many cores it needs to start?

          Now everything works just the opposite way: nodes create the cores they are configured to do (solr.xml/CoreAdminHandler) and the overseer just assigns them a shard id as they come in. I assume everything could still work this way when using the placeholders but it sounds a bit awkward.

          I imagine the placeholders should be both in the ZK structure under /collections and in clusterstate

          What did you have in mind that should be stored under /collections?

          In earlier versions I had overseer read the target number of slices from the collection node (/collections/collection1 for example) but that was later removed in favor of the system property.

          What should be in charge of creating a new collection? Now you can do it for example through CoreAdminHandler by simply adding a core into a collection that does not yet exist, I believe you can do this also through editing solr.xml.

          Show
          Sami Siren added a comment - the cluster should be self-describing so that one can start up a node and say "go join this cluster" Do you mean that the new node would automatically discover from the placeholders how many cores it needs to start? Now everything works just the opposite way: nodes create the cores they are configured to do (solr.xml/CoreAdminHandler) and the overseer just assigns them a shard id as they come in. I assume everything could still work this way when using the placeholders but it sounds a bit awkward. I imagine the placeholders should be both in the ZK structure under /collections and in clusterstate What did you have in mind that should be stored under /collections? In earlier versions I had overseer read the target number of slices from the collection node (/collections/collection1 for example) but that was later removed in favor of the system property. What should be in charge of creating a new collection? Now you can do it for example through CoreAdminHandler by simply adding a core into a collection that does not yet exist, I believe you can do this also through editing solr.xml.
          Hide
          Yonik Seeley added a comment -

          I think the only change I'm suggesting is that when a new collection is created via bootstrapping and the -DnumShards=3 parameter is passed,
          that empty placeholders for shard2 and shard3 be created:

          {"collection1":{"shard1":{
                "Rogue:8983_solr_":{
                  "shard_id":"shard1",
                  "leader":"true",
                  "state":"active",
                  "core":"",
                  "collection":"collection1",
                  "node_name":"Rogue:8983_solr",
                  "base_url":"http://Rogue:8983/solr"}},
            "shard2":{}
            "shard3":{}
          }
          

          This allows for appropriate errors to be thrown when shards that would normally be needed are not "up".

          All new nodes being brought up can now omit the -DnumShards parameter... and the overseer assigns as it normally would.
          We should shoot for being able to have pretty dumb nodes on startup that are only told to join a cluster and nothing else... the cluster controls everything.

          Do you mean that the new node would automatically discover from the placeholders how many cores it needs to start?

          No... by default it would still only be one core. Although we could have a parameter that would start more than one core.

          What did you have in mind that should be stored under /collections?

          There could be a placeholder for shard2, but if the only thing that uses it is leader election, and it's always created on demand, then it shouldn't be necessary.

          In earlier versions I had overseer read the target number of slices from the collection node (/collections/collection1 for example) but that was later removed in favor of the system property.

          Right - that still sounds correct as it will fit better with custom sharding and with shard splitting (however that will work).

          What should be in charge of creating a new collection? Now you can do it for example through CoreAdminHandler by simply adding a core into a collection that does not yet exist

          We should eventually have some API to create a new collection without necessitating the creation of new nodes. Perhaps it's part of the core admin handler? Perhaps Mark has thoughts on this.

          Show
          Yonik Seeley added a comment - I think the only change I'm suggesting is that when a new collection is created via bootstrapping and the -DnumShards=3 parameter is passed, that empty placeholders for shard2 and shard3 be created: { "collection1" :{ "shard1" :{ "Rogue:8983_solr_" :{ "shard_id" : "shard1" , "leader" : " true " , "state" : "active" , "core" :"", "collection" : "collection1" , "node_name" : "Rogue:8983_solr" , "base_url" : "http: //Rogue:8983/solr" }}, "shard2" :{} "shard3" :{} } This allows for appropriate errors to be thrown when shards that would normally be needed are not "up". All new nodes being brought up can now omit the -DnumShards parameter... and the overseer assigns as it normally would. We should shoot for being able to have pretty dumb nodes on startup that are only told to join a cluster and nothing else... the cluster controls everything. Do you mean that the new node would automatically discover from the placeholders how many cores it needs to start? No... by default it would still only be one core. Although we could have a parameter that would start more than one core. What did you have in mind that should be stored under /collections? There could be a placeholder for shard2, but if the only thing that uses it is leader election, and it's always created on demand, then it shouldn't be necessary. In earlier versions I had overseer read the target number of slices from the collection node (/collections/collection1 for example) but that was later removed in favor of the system property. Right - that still sounds correct as it will fit better with custom sharding and with shard splitting (however that will work). What should be in charge of creating a new collection? Now you can do it for example through CoreAdminHandler by simply adding a core into a collection that does not yet exist We should eventually have some API to create a new collection without necessitating the creation of new nodes. Perhaps it's part of the core admin handler? Perhaps Mark has thoughts on this.
          Hide
          Mark Miller added a comment - - edited

          My thoughts are in line with what Yonik has said.

          I only saw this issue as:

          dummy entries get added to the collection state - eg if you start the first node with -DnumShards=3, one real shard entry is added to the collection state and two dummy entries. As Yonik says, now all searches will fail because the 2 dummy shards are not 'up' and the full collection is not available.

          Again, as Yonik notes, a nice side effect is that you don't always have to pass numShards = except perhaps in the future when you want to reshard if that ends up making sense. Is it a little bit like storing the number of shards as you where? Heh - I guess it is - though with the side affect of nodes being able to see a full view of what shards should be in the collection right away (that is, the info is in the clusterstate.json).

          I've been leaning towards getting the req to pass numShards every time out anyway having dealt with it for a while now. This seems like a slightly more useful way to do it compared to just storing that value though. As long as the system property would always update it, I never had much of a problem with storing the value actually. I had been imagining you would trigger a re shard by passing a new number - but I'm not even sure if that is how that will work out.

          Essentially though, everything would work as it does now - I'm not sure we need to do anything more in this issue?

          Show
          Mark Miller added a comment - - edited My thoughts are in line with what Yonik has said. I only saw this issue as: dummy entries get added to the collection state - eg if you start the first node with -DnumShards=3, one real shard entry is added to the collection state and two dummy entries. As Yonik says, now all searches will fail because the 2 dummy shards are not 'up' and the full collection is not available. Again, as Yonik notes, a nice side effect is that you don't always have to pass numShards = except perhaps in the future when you want to reshard if that ends up making sense. Is it a little bit like storing the number of shards as you where? Heh - I guess it is - though with the side affect of nodes being able to see a full view of what shards should be in the collection right away (that is, the info is in the clusterstate.json). I've been leaning towards getting the req to pass numShards every time out anyway having dealt with it for a while now. This seems like a slightly more useful way to do it compared to just storing that value though. As long as the system property would always update it, I never had much of a problem with storing the value actually. I had been imagining you would trigger a re shard by passing a new number - but I'm not even sure if that is how that will work out. Essentially though, everything would work as it does now - I'm not sure we need to do anything more in this issue?
          Hide
          Sami Siren added a comment -

          Ok, I think I got it now. Looks like I was perhaps thinking too far ahead...

          Show
          Sami Siren added a comment - Ok, I think I got it now. Looks like I was perhaps thinking too far ahead...
          Hide
          Yonik Seeley added a comment -

          The mental model I've had is that numShards is just an optional helper at collection creation time, and is largely meaningless after that point. clusterstate is the system of record and contains all of the shards in a collection.
          When shards are split in the future there will be a period of time when both the pre-split shard and the resulting post-split shards will coexist and overlap, and numShards as a collection property (rather than just a creation property) doesn't make sense.
          If one is going to do custom sharding, then one would not use numShards, but simply add shards as needed (an example is shard-by-date, adding a new shard every week).

          Show
          Yonik Seeley added a comment - The mental model I've had is that numShards is just an optional helper at collection creation time, and is largely meaningless after that point. clusterstate is the system of record and contains all of the shards in a collection. When shards are split in the future there will be a period of time when both the pre-split shard and the resulting post-split shards will coexist and overlap, and numShards as a collection property (rather than just a creation property) doesn't make sense. If one is going to do custom sharding, then one would not use numShards, but simply add shards as needed (an example is shard-by-date, adding a new shard every week).
          Hide
          Sami Siren added a comment -

          and numShards as a collection property (rather than just a creation property) doesn't make sense.

          Yeah I agree. I just got confused when you mentioned (in the original issue text) that something would have to be stored under /collections in zk.

          So to summarize the scope of this issue once more:

          Initiating the cluster/collection from the event of starting the first node:

          The first node that joins the cluster will pass the desired slice count (numShards System Property) and that number will be used to generate the placeholders (slices containing 0 shards) for the specific collection in cloudstate.

          After that everything works pretty much as it is now, with one exception: overseer does not look at the system property for determining how many slices there should be but uses the slice count (the placeholders in the beginning) from cloudstate.

          Show
          Sami Siren added a comment - and numShards as a collection property (rather than just a creation property) doesn't make sense. Yeah I agree. I just got confused when you mentioned (in the original issue text) that something would have to be stored under /collections in zk. So to summarize the scope of this issue once more: Initiating the cluster/collection from the event of starting the first node: The first node that joins the cluster will pass the desired slice count (numShards System Property) and that number will be used to generate the placeholders (slices containing 0 shards) for the specific collection in cloudstate. After that everything works pretty much as it is now, with one exception: overseer does not look at the system property for determining how many slices there should be but uses the slice count (the placeholders in the beginning) from cloudstate.
          Hide
          Sami Siren added a comment -

          with one exception: overseer does not look at the system property for determining how many slices there should be but uses the slice count (the placeholders in the beginning) from cloudstate.

          ...actually the shard assignment seems to do this already, it looks at the number of existing slices from cloud state.

          Show
          Sami Siren added a comment - with one exception: overseer does not look at the system property for determining how many slices there should be but uses the slice count (the placeholders in the beginning) from cloudstate. ...actually the shard assignment seems to do this already, it looks at the number of existing slices from cloud state.
          Hide
          Yonik Seeley added a comment -

          The first node that joins the cluster will pass the desired slice count (numShards System Property) and that number will be used to generate the placeholders

          I wouldn't model it as "first node passes the desired slice count" though. I'd model it as "collection creation has an optional numShards param".
          Collection creation happens to always coincide with "first node that joins" just because we don't yet have a separate create-collection API?

          Show
          Yonik Seeley added a comment - The first node that joins the cluster will pass the desired slice count (numShards System Property) and that number will be used to generate the placeholders I wouldn't model it as "first node passes the desired slice count" though. I'd model it as "collection creation has an optional numShards param". Collection creation happens to always coincide with "first node that joins" just because we don't yet have a separate create-collection API?
          Hide
          Sami Siren added a comment -

          Yeah, I'd imagine that by using the other available way to create new collection (CoreAdminHandler) you'd pass the desired number of slices as an additional parameter to the create action.

          Show
          Sami Siren added a comment - Yeah, I'd imagine that by using the other available way to create new collection (CoreAdminHandler) you'd pass the desired number of slices as an additional parameter to the create action.
          Hide
          Mark Miller added a comment -

          I've actually been working on that right now because I need it for a test I'm writing.

          What I realized pretty much immediately was that I needed a way to pass that param down the chain so that the overseer eventually sees it. You should double check what I have done probably. What I found is that it seems only the overseer looks at the numShards sys prop - so it won't matter if its set on other nodes?

          Anyway, what I settled for at the moment is, you can set the numShards on the cloud descriptor - its not stored, so you may not always get it back in the future, it's just a startup param. Then num_shards is also registered as part of the core state in zk - again it's only needed once though. This lets us specify numShards per core though - which is kind of what you need to support it in CoreAdminHandler.

          Because the numShard will stay in the core_state, I'm not yet sure how that will work if you end up being able to reshard by starting a core with a different number...

          Show
          Mark Miller added a comment - I've actually been working on that right now because I need it for a test I'm writing. What I realized pretty much immediately was that I needed a way to pass that param down the chain so that the overseer eventually sees it. You should double check what I have done probably. What I found is that it seems only the overseer looks at the numShards sys prop - so it won't matter if its set on other nodes? Anyway, what I settled for at the moment is, you can set the numShards on the cloud descriptor - its not stored, so you may not always get it back in the future, it's just a startup param. Then num_shards is also registered as part of the core state in zk - again it's only needed once though. This lets us specify numShards per core though - which is kind of what you need to support it in CoreAdminHandler. Because the numShard will stay in the core_state, I'm not yet sure how that will work if you end up being able to reshard by starting a core with a different number...
          Hide
          Mark Miller added a comment -

          To clarify:

          I've actually been working on that right now

          By that, I mean making it so that you can pass numShards via the create action with CoreAdminHandler - for a test for SOLR-3108

          Show
          Mark Miller added a comment - To clarify: I've actually been working on that right now By that, I mean making it so that you can pass numShards via the create action with CoreAdminHandler - for a test for SOLR-3108
          Hide
          Sami Siren added a comment -

          Initial patch. Once the changes in SOLR-3108 gets committed this patch needs to be updated to so that the System property is no longer used.

          Show
          Sami Siren added a comment - Initial patch. Once the changes in SOLR-3108 gets committed this patch needs to be updated to so that the System property is no longer used.
          Hide
          Sami Siren added a comment -

          Also i think the error message that is displayed when querying a collection with placeholders needs to be improved. it's not that easy to understand what's going on from:

          Caused by: org.apache.solr.common.SolrException: no servers hosting shard:
          
          no servers hosting shard:
          
          request: http://localhost:33529/solr/collection1/select?q=*:*&wt=javabin&version=2
          	at org.apache.solr.client.solrj.impl.CommonsHttpSolrServer.request(CommonsHttpSolrServer.java:433)
          	at org.apache.solr.client.solrj.impl.CommonsHttpSolrServer.request(CommonsHttpSolrServer.java:251)
          	at org.apache.solr.client.solrj.request.QueryRequest.process(QueryRequest.java:89)
          	... 34 more
          
          Show
          Sami Siren added a comment - Also i think the error message that is displayed when querying a collection with placeholders needs to be improved. it's not that easy to understand what's going on from: Caused by: org.apache.solr.common.SolrException: no servers hosting shard: no servers hosting shard: request: http: //localhost:33529/solr/collection1/select?q=*:*&wt=javabin&version=2 at org.apache.solr.client.solrj.impl.CommonsHttpSolrServer.request(CommonsHttpSolrServer.java:433) at org.apache.solr.client.solrj.impl.CommonsHttpSolrServer.request(CommonsHttpSolrServer.java:251) at org.apache.solr.client.solrj.request.QueryRequest.process(QueryRequest.java:89) ... 34 more
          Hide
          Mark Miller added a comment -

          Also i think the error message that is displayed when querying a collection with placeholders needs to be improved.

          +1 - I saw this a lot during dev and always had to jump into the code again to figure out what the heck it meant (no shard is even listed, so its just super confusing)

          Show
          Mark Miller added a comment - Also i think the error message that is displayed when querying a collection with placeholders needs to be improved. +1 - I saw this a lot during dev and always had to jump into the code again to figure out what the heck it meant (no shard is even listed, so its just super confusing)
          Hide
          Sami Siren added a comment -

          an updated patch that applies with current trunk

          Show
          Sami Siren added a comment - an updated patch that applies with current trunk
          Hide
          Mark Miller added a comment -

          Thanks Sami! I've committed your latest patch.

          Show
          Mark Miller added a comment - Thanks Sami! I've committed your latest patch.
          Hide
          Sami Siren added a comment -

          I saw this a lot during dev and always had to jump into the code again to figure out what the heck it meant (no shard is even listed, so its just super confusing)

          I added a new issue for this: SOLR-3118

          Show
          Sami Siren added a comment - I saw this a lot during dev and always had to jump into the code again to figure out what the heck it meant (no shard is even listed, so its just super confusing) I added a new issue for this: SOLR-3118
          Hide
          Mark Miller added a comment -

          You done here Sami? Should we resolve this one?

          Show
          Mark Miller added a comment - You done here Sami? Should we resolve this one?
          Hide
          Sami Siren added a comment -

          Yeah, this can be resolved.

          Show
          Sami Siren added a comment - Yeah, this can be resolved.
          Hide
          Uwe Schindler added a comment -

          Closed after release.

          Show
          Uwe Schindler added a comment - Closed after release.

            People

            • Assignee:
              Sami Siren
              Reporter:
              Yonik Seeley
            • Votes:
              0 Vote for this issue
              Watchers:
              1 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved:

                Development