[
https://issues.apache.org/jira/browse/HBASE-22804?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16904184#comment-16904184
]
Mingliang Liu edited comment on HBASE-22804 at 8/10/19 3:54 AM:
----------------------------------------------------------------
Nits:
# {{regionMap}} is created at construction time, and it will not be set null.
So in test {{assertNotNull("verify region map exists", regionMap);}} seems not
necessary. That's said, we can also make {{regionMap}} final.
# I think it's bit clearer to have newly added counters in separate lines.
{code}
149 writeFailureCount = new AtomicLong(0),
150 readSuccessCount = new AtomicLong(0),
151 writeSuccessCount = new AtomicLong(0);
{code}
to
{code}
private final AtomicLong writeFailureCount = new AtomicLong(0);
private final AtomicLong readSuccessCount = new AtomicLong(0);
private final AtomicLong writeSuccessCount = new AtomicLong(0);
{code}
# You can focus on {{master}} branch first, and after ready for commit to
prepare a {{branch-1}}/{{branch-2}} patch if it does not apply cleanly. This
can save a little bit time.
# {{TableName tableName = TableName.valueOf("testTable");}} Test table name
can be a variable and be referenced later. The value can be the test method
name to avoid potential conflict.
{code}
final String tableName = name.getMethodName();
Table table = testingUtility.createTable(TableName.valueOf(tableName), new
byte[][] { FAMILY });
...
String[] args = { "-writeSniffing", "-t", "10000", tableName };
{code}
# {{for (Map.Entry<String, Canary.RegionTaskResult> entry :
regionMap.entrySet())}} When iterating a Map you can consider using simpler
format {{for (String regionName : regionMap.keySet())}}.
Question:
{{private Map<String, RegionTaskResult> regionMap = Maps.newConcurrentMap();}}
can be replaced with {{private Map<String, RegionTaskResult> regionMap = new
ConcurrentHashMap<>();}} to not use guava? Actually I'm thinking do we need it
to be concurrent Map? We populate all regions before the sniff and then all
other places simply get the individual item to update. Thoughts?
was (Author: liuml07):
Nits:
# {{regionMap}} is created at construction time, and it will be set null. So in
test {{assertNotNull("verify region map exists", regionMap);}} seems not
necessary. That's said, we can also make {{regionMap}} final.
# I think it's bit clearer to have newly added counters in separate lines.
{code}
149 writeFailureCount = new AtomicLong(0),
150 readSuccessCount = new AtomicLong(0),
151 writeSuccessCount = new AtomicLong(0);
{code}
to
{code}
private final AtomicLong writeFailureCount = new AtomicLong(0);
private final AtomicLong readSuccessCount = new AtomicLong(0);
private final AtomicLong writeSuccessCount = new AtomicLong(0);
{code}
# You can focus on {{master}} branch first, and after ready for commit to
prepare a {{branch-1}}/{{branch-2}} patch if it does not apply cleanly. This
can save a little bit time.
# {{TableName tableName = TableName.valueOf("testTable");}} Test table name
can be a variable and be referenced later. The value can be the test method
name to avoid potential conflict.
{code}
final String tableName = name.getMethodName();
Table table = testingUtility.createTable(TableName.valueOf(tableName), new
byte[][] { FAMILY });
...
String[] args = { "-writeSniffing", "-t", "10000", tableName };
{code}
# {{for (Map.Entry<String, Canary.RegionTaskResult> entry :
regionMap.entrySet())}} When iterating a Map you can consider using simpler
format {{for (String regionName : regionMap.keySet())}}.
Question:
{{private Map<String, RegionTaskResult> regionMap = Maps.newConcurrentMap();}}
can be replaced with {{private Map<String, RegionTaskResult> regionMap = new
ConcurrentHashMap<>();}} to not use guava? Actually I'm thinking do we need it
to be concurrent Map? We populate all regions before the sniff and then all
other places simply get the individual item to update, right? If so I think
making it {{final}} will be enough. Thoughts?
> Provide an API to get list of successful regions and total expected regions
> in Canary
> -------------------------------------------------------------------------------------
>
> Key: HBASE-22804
> URL: https://issues.apache.org/jira/browse/HBASE-22804
> Project: HBase
> Issue Type: Improvement
> Components: canary
> Affects Versions: 3.0.0, 1.3.0, 1.4.0, 1.5.0, 2.0.0, 2.1.5, 2.2.1
> Reporter: Caroline
> Assignee: Caroline
> Priority: Minor
> Labels: Canary
> Attachments: HBASE-22804.branch-1.001.patch,
> HBASE-22804.branch-2.001.patch, HBASE-22804.master.001.patch
>
>
> At present HBase Canary tool only prints the successes as part of logs.
> Providing an API to get the list of successes, as well as total number of
> expected regions, will make it easier to get a more accurate availability
> estimate.
>
--
This message was sent by Atlassian JIRA
(v7.6.14#76016)
