lucene-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Mark Miller (JIRA)" <j...@apache.org>
Subject [jira] [Comment Edited] (SOLR-5473) Make one state.json per collection
Date Thu, 24 Apr 2014 01:33:18 GMT

    [ https://issues.apache.org/jira/browse/SOLR-5473?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13979177#comment-13979177
] 

Mark Miller edited comment on SOLR-5473 at 4/24/14 1:31 AM:
------------------------------------------------------------

bq. externalWatchedCollections

What are these? Could we get a comment to describe?

bq. getExternCollectionFresh

What is this? Again, I think we need something better than Extern(sp?) and Fresh, but the
method itself also has no documentation.

bq.   * <b>Advance usage</b>

The project tends to use Expert or Expert method, and a lot of these new methods should probably
have been marked lucene.experimental. Also, appears to be a typo.

bq. private Map<String , DocCollection>
bq.  if(zkStateReader.ephemeralCollectionData !=null ){
bq.  return  cs.getCommonCollection(coll);

A lot of your code that gets committed has odd formatting - would be great to use one of the
eclipse or intellij code profiles we have for formatting.

{noformat}
   * This method can be used to fetch a collection object and control whether it hits
   * the cache only or if information can be looked up from ZooKeeper.
{noformat}

I brought this up before and it didn't seem to be address in your patch? I don't know how
a user can understand this - you could always get the latest collection info by doing zkStateReader#updateState
and then getCollecttion. What does this method offer that is different that is worth the API
ugliness? 

{code}
    } catch (InterruptedException e) {
      throw new SolrException(ErrorCode.BAD_REQUEST, "Could not load collection from ZK:"
+ coll, e);
    }
{code}

When you catch an InterruptedException you should do Thread.currenThread.interrupt() to reset
the flag.

{code}
  /**This is not a public API. Only used by ZkController */
  public void removeZKWatch(final String coll){
{code}

Should be marked internal then. Not a great design that has public internal methods on public
objects though.


was (Author: markrmiller@gmail.com):
bq. externalWatchedCollections

What are these? Could we get a comment to describe?

bq. getExternCollectionFresh

What is this? Again, I think we need something better than Extern(sp?) and Fresh, but the
method itself also has no documentation.

bq.   * <b>Advance usage</b>

The project tends to use Expert or Expert method, and a lot of these new methods should probably
have been marked lucene.experimental. Also, appears to be a typo.

bq. private Map<String , DocCollection>
bq.  if(zkStateReader.ephemeralCollectionData !=null ){
bq.  return  cs.getCommonCollection(coll);

A lot of your code that gets committed has odd formatting - would be great to use one of the
eclipse or intellij code profiles we have for formatting.

{noformat}
   * This method can be used to fetch a collection object and control whether it hits
   * the cache only or if information can be looked up from ZooKeeper.
{noformat}

I brought this up before and it didn't seem to be address in your patch? I don't know how
a user can understand this - clusterState

{code}
    } catch (InterruptedException e) {
      throw new SolrException(ErrorCode.BAD_REQUEST, "Could not load collection from ZK:"
+ coll, e);
    }
{code}

When you catch an InterruptedException you should do Thread.currenThread.interrupt() to reset
the flag.

{code}
  /**This is not a public API. Only used by ZkController */
  public void removeZKWatch(final String coll){
{code}

Should be marked internal then. Not a great design that has public internal methods on public
objects though.

> Make one state.json per collection
> ----------------------------------
>
>                 Key: SOLR-5473
>                 URL: https://issues.apache.org/jira/browse/SOLR-5473
>             Project: Solr
>          Issue Type: Sub-task
>          Components: SolrCloud
>            Reporter: Noble Paul
>            Assignee: Noble Paul
>             Fix For: 5.0
>
>         Attachments: SOLR-5473-74.patch, SOLR-5473-74.patch, SOLR-5473-74.patch, SOLR-5473-74.patch,
SOLR-5473-74.patch, SOLR-5473-74.patch, SOLR-5473-74.patch, SOLR-5473-74.patch, SOLR-5473-74.patch,
SOLR-5473-74.patch, SOLR-5473-74.patch, SOLR-5473-74.patch, SOLR-5473-74.patch, SOLR-5473-74.patch,
SOLR-5473-74.patch, SOLR-5473-74.patch, SOLR-5473-74.patch, SOLR-5473-74.patch, SOLR-5473-74.patch,
SOLR-5473-74.patch, SOLR-5473-74.patch, SOLR-5473-74.patch, SOLR-5473-74.patch, SOLR-5473-74.patch,
SOLR-5473.patch, SOLR-5473.patch, SOLR-5473.patch, SOLR-5473.patch, SOLR-5473.patch, SOLR-5473.patch,
SOLR-5473.patch, SOLR-5473.patch, SOLR-5473.patch, SOLR-5473.patch, SOLR-5473.patch, SOLR-5473.patch,
SOLR-5473.patch, SOLR-5473.patch, SOLR-5473.patch, SOLR-5473.patch, SOLR-5473.patch, SOLR-5473.patch,
ec2-23-20-119-52_solr.log, ec2-50-16-38-73_solr.log
>
>
> As defined in the parent issue, store the states of each collection under /collections/collectionname/state.json
node



--
This message was sent by Atlassian JIRA
(v6.2#6252)

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@lucene.apache.org
For additional commands, e-mail: dev-help@lucene.apache.org


Mime
View raw message