You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@geode.apache.org by nabarun nag <nn...@pivotal.io> on 2017/01/12 02:49:53 UTC
Review Request 55442: GEODE-2273: Adding a server column to the
Lucene index stat
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/55442/
-----------------------------------------------------------
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
Re: Review Request 55442: GEODE-2273: Adding a server column to the
Lucene index stat
Posted by xiaojian zhou <zh...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/55442/#review161458
-----------------------------------------------------------
geode-lucene/src/main/java/org/apache/geode/cache/lucene/internal/cli/LuceneIndexDetails.java (line 48)
<https://reviews.apache.org/r/55442/#comment232726>
How you know which server is for which index's detail?
For example, you will show 2 lines for the same index, but which line is for which server?
- xiaojian zhou
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
>
>
Re: Review Request 55442: GEODE-2273: Adding a server column to the
Lucene index stat
Posted by xiaojian zhou <zh...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/55442/#review161800
-----------------------------------------------------------
Ship it!
Ship It!
- xiaojian zhou
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
>
>
Re: Review Request 55442: GEODE-2273: Adding a server column to the
Lucene index stat
Posted by nabarun nag <nn...@pivotal.io>.
-----------------------------------------------------------
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 (updated)
-------
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
Re: Review Request 55442: GEODE-2273: Adding a server column to the
Lucene index stat
Posted by nabarun nag <nn...@pivotal.io>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/55442/
-----------------------------------------------------------
(Updated Jan. 16, 2017, 7:14 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
```
Diffs (updated)
-----
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
Re: Review Request 55442: GEODE-2273: Adding a server column to the
Lucene index stat
Posted by nabarun nag <nn...@pivotal.io>.
> 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
>
>
Re: Review Request 55442: GEODE-2273: Adding a server column to the
Lucene index stat
Posted by Dan Smith <ds...@pivotal.io>.
-----------------------------------------------------------
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
>
>