Skip to content

Commit 0918145

Browse files
ankitsinghalankitsinghal
committed
Addressing Review comments from Toshi and Josh
1 parent b6f560b commit 0918145

File tree

9 files changed

+126
-22
lines changed

9 files changed

+126
-22
lines changed

‎hbase-hbtop/src/main/java/org/apache/hadoop/hbase/hbtop/mode/ClientModeStrategy.java‎

Lines changed: 11 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,7 @@
1919

2020
import java.util.ArrayList;
2121
import java.util.Arrays;
22+
import java.util.Collections;
2223
import java.util.HashMap;
2324
import java.util.HashSet;
2425
import java.util.List;
@@ -62,19 +63,19 @@
6263

6364
@Override public List<Record> getRecords(ClusterMetrics clusterMetrics,
6465
List<RecordFilter> pushDownFilters) {
65-
List<Record> records = createRecords(clusterMetrics, pushDownFilters);
66+
List<Record> records = createRecords(clusterMetrics);
6667
return aggregateRecordsAndAddDistinct(
6768
ModeStrategyUtils.applyFilterAndGet(records, pushDownFilters), Field.CLIENT, Field.USER,
6869
Field.USER_COUNT);
6970
}
7071

71-
List<Record> createRecords(ClusterMetrics clusterMetrics, List<RecordFilter> pushDownFilters) {
72+
List<Record> createRecords(ClusterMetrics clusterMetrics) {
7273
List<Record> ret = new ArrayList<>();
7374
for (ServerMetrics serverMetrics : clusterMetrics.getLiveServerMetrics().values()) {
7475
long lastReportTimestamp = serverMetrics.getLastReportTimestamp();
75-
serverMetrics.getUserMetrics().entrySet().forEach(
76-
um -> um.getValue().getClientMetrics().values().forEach(clientMetrics -> ret.add(
77-
createRecord(um.getValue().getNameAsString(), clientMetrics, lastReportTimestamp))));
76+
serverMetrics.getUserMetrics().values().forEach(
77+
um -> um.getClientMetrics().values().forEach(clientMetrics -> ret.add(
78+
createRecord(um.getNameAsString(), clientMetrics, lastReportTimestamp))));
7879
}
7980
return ret;
8081
}
@@ -91,11 +92,11 @@ List<Record> createRecords(ClusterMetrics clusterMetrics, List<RecordFilter> pus
9192
List<Record> aggregateRecordsAndAddDistinct(List<Record> records, Field groupBy,
9293
Field distinctField, Field uniqueCountAssignedTo) {
9394
List<Record> result = new ArrayList<>();
94-
records.stream().collect(Collectors.groupingBy(r -> r.get(groupBy))).entrySet()
95-
.forEach(entry -> {
95+
records.stream().collect(Collectors.groupingBy(r -> r.get(groupBy))).values()
96+
.forEach(val -> {
9697
Set<FieldValue> distinctValues = new HashSet<>();
97-
Map<Field, FieldValue> map = new HashMap();
98-
for (Record record : entry.getValue()) {
98+
Map<Field, FieldValue> map = new HashMap<>();
99+
for (Record record : val) {
99100
for (Map.Entry<Field, FieldValue> field : record.entrySet()) {
100101
if (distinctField.equals(field.getKey())) {
101102
//We will not be adding the field in the new record whose distinct count is required
@@ -143,7 +144,7 @@ Record createRecord(String user, UserMetrics.ClientMetrics clientMetrics,
143144
}
144145

145146
@Override public DrillDownInfo drillDown(Record selectedRecord) {
146-
List<RecordFilter> initialFilters = Arrays.asList(
147+
List<RecordFilter> initialFilters = Collections.singletonList(
147148
RecordFilter.newBuilder(Field.CLIENT).doubleEquals(selectedRecord.get(Field.CLIENT)));
148149
return new DrillDownInfo(Mode.USER, initialFilters);
149150
}

‎hbase-hbtop/src/main/java/org/apache/hadoop/hbase/hbtop/mode/ModeStrategy.java‎

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -34,7 +34,6 @@
3434
interface ModeStrategy {
3535
List<FieldInfo> getFieldInfos();
3636
Field getDefaultSortField();
37-
List<Record> getRecords(ClusterMetrics clusterMetrics,
38-
@Nullable List<RecordFilter> pushDownFilters);
37+
List<Record> getRecords(ClusterMetrics clusterMetrics, List<RecordFilter> pushDownFilters);
3938
@Nullable DrillDownInfo drillDown(Record selectedRecord);
4039
}

‎hbase-hbtop/src/main/java/org/apache/hadoop/hbase/hbtop/mode/UserModeStrategy.java‎

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@
1818
package org.apache.hadoop.hbase.hbtop.mode;
1919

2020
import java.util.Arrays;
21+
import java.util.Collections;
2122
import java.util.List;
2223

2324
import org.apache.hadoop.hbase.ClusterMetrics;
@@ -53,16 +54,16 @@
5354

5455
@Override public List<Record> getRecords(ClusterMetrics clusterMetrics,
5556
List<RecordFilter> pushDownFilters) {
56-
List<Record> records = clientModeStrategy.createRecords(clusterMetrics, pushDownFilters);
57+
List<Record> records = clientModeStrategy.createRecords(clusterMetrics);
5758
return clientModeStrategy.aggregateRecordsAndAddDistinct(
5859
ModeStrategyUtils.applyFilterAndGet(records, pushDownFilters), Field.USER, Field.CLIENT,
5960
Field.CLIENT_COUNT);
6061
}
6162

6263
@Override public DrillDownInfo drillDown(Record selectedRecord) {
6364
//Drill down to client and using selected USER as a filter
64-
List<RecordFilter> initialFilters = Arrays
65-
.asList(RecordFilter.newBuilder(Field.USER).doubleEquals(selectedRecord.get(Field.USER)));
65+
List<RecordFilter> initialFilters = Collections.singletonList(
66+
RecordFilter.newBuilder(Field.USER).doubleEquals(selectedRecord.get(Field.USER)));
6667
return new DrillDownInfo(Mode.CLIENT, initialFilters);
6768
}
6869
}

‎hbase-hbtop/src/main/java/org/apache/hadoop/hbase/hbtop/screen/top/TopScreenModel.java‎

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -156,7 +156,6 @@ public boolean addFilter(String filterString, boolean ignoreCase) {
156156
if (filter == null) {
157157
return false;
158158
}
159-
160159
filters.add(filter);
161160
decomposePushDownFilter();
162161
filterHistories.add(filterString);

‎hbase-hbtop/src/main/java/org/apache/hadoop/hbase/hbtop/screen/top/TopScreenPresenter.java‎

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -327,5 +327,4 @@ public ScreenView goToFilterDisplayMode(Screen screen, Terminal terminal, int ro
327327
return new FilterDisplayModeScreenView(screen, terminal, row, topScreenModel.getFilters(),
328328
topScreenView);
329329
}
330-
331330
}

‎hbase-hbtop/src/test/java/org/apache/hadoop/hbase/hbtop/TestUtils.java‎

Lines changed: 37 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -353,6 +353,31 @@ public static void assertRecordsInUserMode(List<Record> records) {
353353
}
354354
}
355355

356+
public static void assertRecordsInClientMode(List<Record> records) {
357+
assertThat(records.size(), is(4));
358+
for (Record record : records) {
359+
String client = record.get(Field.CLIENT).asString();
360+
switch (client) {
361+
//readRequestPerSecond and writeRequestPerSecond will be zero
362+
// because there is no change or new metrics during refresh
363+
case "CLIENT_A_FOO":
364+
assertRecordInClientMode(record, 0L, 0L);
365+
break;
366+
case "CLIENT_A_BAR":
367+
assertRecordInClientMode(record, 0L, 0L);
368+
break;
369+
case "CLIENT_B_FOO":
370+
assertRecordInClientMode(record, 0L, 0L);
371+
break;
372+
case "CLIENT_B_BAR":
373+
assertRecordInClientMode(record, 0L, 0L);
374+
break;
375+
default:
376+
fail();
377+
}
378+
}
379+
}
380+
356381
private static void assertRecordInUserMode(Record record,
357382
long readRequestCountPerSecond,
358383
long writeCountRequestPerSecond) {
@@ -365,6 +390,18 @@ private static void assertRecordInUserMode(Record record,
365390
is(2));
366391
}
367392

393+
private static void assertRecordInClientMode(Record record,
394+
long readRequestCountPerSecond,
395+
long writeCountRequestPerSecond) {
396+
assertThat(record.size(), is(5));
397+
assertThat(record.get(Field.READ_REQUEST_COUNT_PER_SECOND).asLong(),
398+
is(readRequestCountPerSecond));
399+
assertThat(record.get(Field.WRITE_REQUEST_COUNT_PER_SECOND).asLong(),
400+
is(writeCountRequestPerSecond));
401+
assertThat(record.get(Field.USER_COUNT).asInt(),
402+
is(1));
403+
}
404+
368405
private static void assertRecordInTableMode(Record record, long requestCountPerSecond,
369406
long readRequestCountPerSecond, long filteredReadRequestCountPerSecond,
370407
long writeCountRequestPerSecond, Size storeFileSize, Size uncompressedStoreFileSize,

‎hbase-hbtop/src/test/java/org/apache/hadoop/hbase/hbtop/mode/ClientModeTest.java‎

Lines changed: 73 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,73 @@
1+
/**
2+
* Licensed to the Apache Software Foundation (ASF) under one
3+
* or more contributor license agreements. See the NOTICE file
4+
* distributed with this work for additional information
5+
* regarding copyright ownership. The ASF licenses this file
6+
* to you under the Apache License, Version 2.0 (the
7+
* "License"); you may not use this file except in compliance
8+
* with the License. You may obtain a copy of the License at
9+
*
10+
* http://www.apache.org/licenses/LICENSE-2.0
11+
*
12+
* Unless required by applicable law or agreed to in writing, software
13+
* distributed under the License is distributed on an "AS IS" BASIS,
14+
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
15+
* See the License for the specific language governing permissions and
16+
* limitations under the License.
17+
*/
18+
package org.apache.hadoop.hbase.hbtop.mode;
19+
20+
import static org.hamcrest.CoreMatchers.is;
21+
import static org.junit.Assert.assertThat;
22+
import static org.junit.Assert.fail;
23+
24+
import java.util.List;
25+
26+
import org.apache.hadoop.hbase.HBaseClassTestRule;
27+
import org.apache.hadoop.hbase.hbtop.Record;
28+
import org.apache.hadoop.hbase.hbtop.TestUtils;
29+
import org.apache.hadoop.hbase.hbtop.field.Field;
30+
import org.apache.hadoop.hbase.testclassification.SmallTests;
31+
import org.junit.ClassRule;
32+
import org.junit.experimental.categories.Category;
33+
34+
@Category(SmallTests.class)
35+
public class ClientModeTest extends ModeTestBase {
36+
37+
@ClassRule public static final HBaseClassTestRule CLASS_RULE =
38+
HBaseClassTestRule.forClass(ClientModeTest.class);
39+
40+
@Override protected Mode getMode() {
41+
return Mode.CLIENT;
42+
}
43+
44+
@Override protected void assertRecords(List<Record> records) {
45+
TestUtils.assertRecordsInClientMode(records);
46+
}
47+
48+
@Override protected void assertDrillDown(Record currentRecord, DrillDownInfo drillDownInfo) {
49+
assertThat(drillDownInfo.getNextMode(), is(Mode.USER));
50+
assertThat(drillDownInfo.getInitialFilters().size(), is(1));
51+
String client = currentRecord.get(Field.CLIENT).asString();
52+
53+
switch (client) {
54+
case "CLIENT_A_FOO":
55+
assertThat(drillDownInfo.getInitialFilters().get(0).toString(), is("CLIENT==CLIENT_A_FOO"));
56+
break;
57+
58+
case "CLIENT_B_FOO":
59+
assertThat(drillDownInfo.getInitialFilters().get(0).toString(), is("CLIENT==CLIENT_B_FOO"));
60+
break;
61+
62+
case "CLIENT_A_BAR":
63+
assertThat(drillDownInfo.getInitialFilters().get(0).toString(), is("CLIENT==CLIENT_A_BAR"));
64+
break;
65+
case "CLIENT_B_BAR":
66+
assertThat(drillDownInfo.getInitialFilters().get(0).toString(), is("CLIENT==CLIENT_B_BAR"));
67+
break;
68+
69+
default:
70+
fail();
71+
}
72+
}
73+
}

‎hbase-protocol-shaded/src/main/protobuf/ClusterStatus.proto‎

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -164,8 +164,6 @@ message UserLoad {
164164

165165
/** Metrics for all clients of a user */
166166
repeated ClientMetrics clientMetrics = 4;
167-
168-
169167
}
170168

171169
message ClientMetrics {
@@ -177,7 +175,6 @@ message ClientMetrics {
177175

178176
/** the current total write requests made from a client */
179177
optional uint64 write_requests_count = 3;
180-
181178
}
182179

183180
/* Server-level protobufs */

‎hbase-server/src/test/java/org/apache/hadoop/hbase/TestClientClusterMetrics.java‎

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -79,8 +79,6 @@ public class TestClientClusterMetrics {
7979
public static void setUpBeforeClass() throws Exception {
8080
Configuration conf = HBaseConfiguration.create();
8181
conf.set(CoprocessorHost.MASTER_COPROCESSOR_CONF_KEY, MyObserver.class.getName());
82-
conf.set("hbase.security.authorization", Boolean.TRUE.toString());
83-
conf.set("hbase.security.exec.permission.checks", Boolean.TRUE.toString());
8482
UTIL = new HBaseTestingUtility(conf);
8583
StartMiniClusterOption option = StartMiniClusterOption.builder()
8684
.numMasters(MASTERS).numRegionServers(SLAVES).numDataNodes(SLAVES).build();

0 commit comments

Comments
 (0)