> On Jan. 12, 2017, 5:26 p.m., Dan Smith wrote: > > 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.
serverName is passed while creating the luceneIndexDetails. - nabarun ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/55442/#review161394 ----------------------------------------------------------- On Jan. 16, 2017, 7:37 p.m., nabarun nag wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/55442/ > ----------------------------------------------------------- > > (Updated Jan. 16, 2017, 7:37 p.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 > ``` > > > After Dan's review. > > ``` > Index Name | Region Path | Server Name | Indexed Fields | Field > Analyzer | Status | Query Executions | Updates | Commits | Documents > ---------- | ----------- | ----------- | ---------------------- | > -------------- | ----------- | ---------------- | ------- | ------- | > --------- > testIndex | /testRegion | server1 | [__REGION_VALUE_FIELD] | {} > | Initialized | 0 | 3 | 3 | 2 > testIndex | /testRegion | server2 | [__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 > > geode-lucene/src/main/java/org/apache/geode/cache/lucene/internal/cli/functions/LuceneDescribeIndexFunction.java > 0ec5b4d > > geode-lucene/src/main/java/org/apache/geode/cache/lucene/internal/cli/functions/LuceneListIndexFunction.java > 8732bba > > geode-lucene/src/test/java/org/apache/geode/cache/lucene/internal/cli/LuceneIndexCommandsJUnitTest.java > dbb80dc > > geode-lucene/src/test/java/org/apache/geode/cache/lucene/internal/cli/functions/LuceneDescribeIndexFunctionJUnitTest.java > 7330e2c > > geode-lucene/src/test/java/org/apache/geode/cache/lucene/internal/cli/functions/LuceneListIndexFunctionJUnitTest.java > 3adc030 > > Diff: https://reviews.apache.org/r/55442/diff/ > > > Testing > ------- > > 1. Precheck > 2. Running it on gfsh terminal > > > Thanks, > > nabarun nag > >