----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/55442/#review161394 -----------------------------------------------------------
I'd say actually get rid of the distributed member id part and just keep the name. Also, I don't think it's good practice to look up a singleton inside the constructor for LuceneIndexDetails. It would be better just pass the server name into this class. Hopefully that could avoid using the singleton and also not rely on the fact that the contructor is invoked on the member you are trying to get this information from. The responsibility of this class should be just do hold information, not lookup stuff. - Dan Smith On Jan. 12, 2017, 2:49 a.m., nabarun nag wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/55442/ > ----------------------------------------------------------- > > (Updated Jan. 12, 2017, 2:49 a.m.) > > > Review request for geode, Barry Oglesby, Jason Huynh, Dan Smith, and xiaojian > zhou. > > > Repository: geode > > > Description > ------- > > Issue: > > Initially: > We could not link the index information to the server storing the index. > > ``` > Index Name | Region Path | Indexed Fields | Field Analyzer | Status > | Query Executions | Updates | Commits | Documents > ---------- | ----------- | ---------------------- | -------------- | > ----------- | ---------------- | ------- | ------- | --------- > testIndex | /testRegion | [__REGION_VALUE_FIELD] | {} | > Initialized | 0 | 3 | 3 | 2 > testIndex | /testRegion | [__REGION_VALUE_FIELD] | {} | > Initialized | 0 | 0 | 0 | 1 > ``` > > > Solution: Added a server column. > ``` > Index Name | Region Path | Server Name | > Indexed Fields | Field Analyzer | Status | Query Executions | > Updates | Commits | Documents > ---------- | ----------- | ------------------------------------------ | > ---------------------- | -------------- | ----------- | ---------------- | > ------- | ------- | --------- > testIndex | /testRegion | server2 -- 10.0.0.8(server2:2814)<v2>:1026 | > [__REGION_VALUE_FIELD] | {} | Initialized | 0 | 0 > | 0 | 2 > testIndex | /testRegion | server1 -- 10.0.0.8(server1:3037)<v4>:1025 | > [__REGION_VALUE_FIELD] | {} | Initialized | 0 | 0 > | 0 | 1 > ``` > > > Diffs > ----- > > > geode-lucene/src/main/java/org/apache/geode/cache/lucene/internal/cli/LuceneIndexCommands.java > 6a5a1e0 > > geode-lucene/src/main/java/org/apache/geode/cache/lucene/internal/cli/LuceneIndexDetails.java > cc96df2 > > Diff: https://reviews.apache.org/r/55442/diff/ > > > Testing > ------- > > 1. Precheck > 2. Running it on gfsh terminal > > > Thanks, > > nabarun nag > >
