[GitHub] [hbase] brfrn169 commented on a change in pull request #647: HBASE-22988 Backport HBASE-11062 "hbtop" to branch-1
brfrn169 commented on a change in pull request #647: HBASE-22988 Backport
HBASE-11062 "hbtop" to branch-1
URL: https://github.com/apache/hbase/pull/647#discussion_r327394703
##
File path:
hbase-hbtop/src/main/java/org/apache/hadoop/hbase/hbtop/screen/Screen.java
##
@@ -0,0 +1,132 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.hadoop.hbase.hbtop.screen;
+
+import java.io.Closeable;
+import java.io.IOException;
+import java.util.concurrent.TimeUnit;
+
+import org.apache.commons.logging.Log;
+import org.apache.commons.logging.LogFactory;
+import org.apache.hadoop.conf.Configuration;
+import org.apache.hadoop.hbase.classification.InterfaceAudience;
+import org.apache.hadoop.hbase.client.Admin;
+import org.apache.hadoop.hbase.client.Connection;
+import org.apache.hadoop.hbase.client.ConnectionFactory;
+import org.apache.hadoop.hbase.hbtop.mode.Mode;
+import org.apache.hadoop.hbase.hbtop.screen.top.TopScreenView;
+import org.apache.hadoop.hbase.hbtop.terminal.KeyPress;
+import org.apache.hadoop.hbase.hbtop.terminal.Terminal;
+import org.apache.hadoop.hbase.hbtop.terminal.impl.TerminalImpl;
+
+/**
+ * This dispatches key presses and timers to the current {@link ScreenView}.
+ */
+@InterfaceAudience.Private
+public class Screen implements Closeable {
+
+ private static final Log LOG = LogFactory.getLog(Screen.class);
+
+ private static final long SLEEP_TIMEOUT_MILLISECONDS = 100;
+
+ private final Connection connection;
+ private final Admin admin;
+ private final Terminal terminal;
+
+ private ScreenView currentScreenView;
+ private Long timerTimestamp;
+
+ public Screen(Configuration conf, long initialRefreshDelay, Mode initialMode)
+throws IOException {
+connection = ConnectionFactory.createConnection(conf);
+admin = connection.getAdmin();
Review comment:
I could connect to a kerberized cluster without any errors after kinit in my
env. Do we need any modification here?
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] [hbase] brfrn169 commented on a change in pull request #647: HBASE-22988 Backport HBASE-11062 "hbtop" to branch-1
brfrn169 commented on a change in pull request #647: HBASE-22988 Backport
HBASE-11062 "hbtop" to branch-1
URL: https://github.com/apache/hbase/pull/647#discussion_r327060764
##
File path:
hbase-hbtop/src/main/java/org/apache/hadoop/hbase/hbtop/RecordFilter.java
##
@@ -0,0 +1,336 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.hadoop.hbase.hbtop;
+
+import java.util.Arrays;
+import java.util.List;
+import java.util.Objects;
+
+import org.apache.hadoop.hbase.classification.InterfaceAudience;
+import org.apache.hadoop.hbase.hbtop.field.Field;
+import org.apache.hadoop.hbase.hbtop.field.FieldValue;
+
+/**
+ * Represents a filter that's filtering the metric {@link Record}s.
+ */
+@InterfaceAudience.Private
+public final class RecordFilter {
+
+ private enum Operator {
+EQUAL("="),
+DOUBLE_EQUALS("=="),
+GREATER(">"),
+GREATER_OR_EQUAL(">="),
+LESS("<"),
+LESS_OR_EQUAL("<=");
+
+private final String operator;
+
+Operator(String operator) {
+ this.operator = operator;
+}
+
+@Override
+public String toString() {
+ return operator;
+}
+ }
+
+ public static RecordFilter parse(String filterString, boolean ignoreCase) {
+return parse(filterString, Arrays.asList(Field.values()), ignoreCase);
+ }
+
+ public static RecordFilter parse(String filterString, List fields,
boolean ignoreCase) {
+int index = 0;
Review comment:
I just filed this issue in https://issues.apache.org/jira/browse/HBASE-23064
.
Thanks.
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] [hbase] brfrn169 commented on a change in pull request #647: HBASE-22988 Backport HBASE-11062 "hbtop" to branch-1
brfrn169 commented on a change in pull request #647: HBASE-22988 Backport
HBASE-11062 "hbtop" to branch-1
URL: https://github.com/apache/hbase/pull/647#discussion_r327057518
##
File path:
hbase-hbtop/src/main/java/org/apache/hadoop/hbase/hbtop/RecordFilter.java
##
@@ -0,0 +1,336 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.hadoop.hbase.hbtop;
+
+import java.util.Arrays;
+import java.util.List;
+import java.util.Objects;
+
+import org.apache.hadoop.hbase.classification.InterfaceAudience;
+import org.apache.hadoop.hbase.hbtop.field.Field;
+import org.apache.hadoop.hbase.hbtop.field.FieldValue;
+
+/**
+ * Represents a filter that's filtering the metric {@link Record}s.
+ */
+@InterfaceAudience.Private
+public final class RecordFilter {
+
+ private enum Operator {
+EQUAL("="),
+DOUBLE_EQUALS("=="),
+GREATER(">"),
+GREATER_OR_EQUAL(">="),
+LESS("<"),
+LESS_OR_EQUAL("<=");
+
+private final String operator;
+
+Operator(String operator) {
+ this.operator = operator;
+}
+
+@Override
+public String toString() {
+ return operator;
+}
+ }
+
+ public static RecordFilter parse(String filterString, boolean ignoreCase) {
+return parse(filterString, Arrays.asList(Field.values()), ignoreCase);
+ }
+
+ public static RecordFilter parse(String filterString, List fields,
boolean ignoreCase) {
+int index = 0;
Review comment:
Sure. It's not only for branch-1 but also master and branch-2s, so I will
file it in a new ticket in Apache Jira. Thanks.
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] [hbase] brfrn169 commented on a change in pull request #647: HBASE-22988 Backport HBASE-11062 "hbtop" to branch-1
brfrn169 commented on a change in pull request #647: HBASE-22988 Backport
HBASE-11062 "hbtop" to branch-1
URL: https://github.com/apache/hbase/pull/647#discussion_r327049130
##
File path:
hbase-hbtop/src/main/java/org/apache/hadoop/hbase/hbtop/RecordFilter.java
##
@@ -0,0 +1,336 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.hadoop.hbase.hbtop;
+
+import java.util.Arrays;
+import java.util.List;
+import java.util.Objects;
+
+import org.apache.hadoop.hbase.classification.InterfaceAudience;
+import org.apache.hadoop.hbase.hbtop.field.Field;
+import org.apache.hadoop.hbase.hbtop.field.FieldValue;
+
+/**
+ * Represents a filter that's filtering the metric {@link Record}s.
+ */
+@InterfaceAudience.Private
+public final class RecordFilter {
+
+ private enum Operator {
+EQUAL("="),
+DOUBLE_EQUALS("=="),
+GREATER(">"),
+GREATER_OR_EQUAL(">="),
+LESS("<"),
+LESS_OR_EQUAL("<=");
+
+private final String operator;
+
+Operator(String operator) {
+ this.operator = operator;
+}
+
+@Override
+public String toString() {
+ return operator;
+}
+ }
+
+ public static RecordFilter parse(String filterString, boolean ignoreCase) {
+return parse(filterString, Arrays.asList(Field.values()), ignoreCase);
+ }
+
+ public static RecordFilter parse(String filterString, List fields,
boolean ignoreCase) {
+int index = 0;
Review comment:
Okay sure.
Actually, at first, I was thinking of using ANTLR to parse filters but the
syntax of filters is very simple so I thought the lib was too much and decided
to use the simplest way (the current logic). Similarly, I think
`index-substring` way and `pattern-matching` way are too much in this case,
right? If we bring in a more complex syntax of filters to hbtop, we can
consider the better ways. What do you think?
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] [hbase] brfrn169 commented on a change in pull request #647: HBASE-22988 Backport HBASE-11062 "hbtop" to branch-1
brfrn169 commented on a change in pull request #647: HBASE-22988 Backport
HBASE-11062 "hbtop" to branch-1
URL: https://github.com/apache/hbase/pull/647#discussion_r327049130
##
File path:
hbase-hbtop/src/main/java/org/apache/hadoop/hbase/hbtop/RecordFilter.java
##
@@ -0,0 +1,336 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.hadoop.hbase.hbtop;
+
+import java.util.Arrays;
+import java.util.List;
+import java.util.Objects;
+
+import org.apache.hadoop.hbase.classification.InterfaceAudience;
+import org.apache.hadoop.hbase.hbtop.field.Field;
+import org.apache.hadoop.hbase.hbtop.field.FieldValue;
+
+/**
+ * Represents a filter that's filtering the metric {@link Record}s.
+ */
+@InterfaceAudience.Private
+public final class RecordFilter {
+
+ private enum Operator {
+EQUAL("="),
+DOUBLE_EQUALS("=="),
+GREATER(">"),
+GREATER_OR_EQUAL(">="),
+LESS("<"),
+LESS_OR_EQUAL("<=");
+
+private final String operator;
+
+Operator(String operator) {
+ this.operator = operator;
+}
+
+@Override
+public String toString() {
+ return operator;
+}
+ }
+
+ public static RecordFilter parse(String filterString, boolean ignoreCase) {
+return parse(filterString, Arrays.asList(Field.values()), ignoreCase);
+ }
+
+ public static RecordFilter parse(String filterString, List fields,
boolean ignoreCase) {
+int index = 0;
Review comment:
Okay sure.
Actually, at first, I was thinking of using ANTLR to parse filters but the
syntax of filters is very simple so I thought the lib was too much and decided
to use a simplest way (the current logic). Similarly, I think
`index-substring` way and `pattern-matching` way are too much in this case,
right? If we bring in a more complex syntax of filters to hbtop, we can
consider the better way. What do you think?
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] [hbase] brfrn169 commented on a change in pull request #647: HBASE-22988 Backport HBASE-11062 "hbtop" to branch-1
brfrn169 commented on a change in pull request #647: HBASE-22988 Backport
HBASE-11062 "hbtop" to branch-1
URL: https://github.com/apache/hbase/pull/647#discussion_r327049130
##
File path:
hbase-hbtop/src/main/java/org/apache/hadoop/hbase/hbtop/RecordFilter.java
##
@@ -0,0 +1,336 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.hadoop.hbase.hbtop;
+
+import java.util.Arrays;
+import java.util.List;
+import java.util.Objects;
+
+import org.apache.hadoop.hbase.classification.InterfaceAudience;
+import org.apache.hadoop.hbase.hbtop.field.Field;
+import org.apache.hadoop.hbase.hbtop.field.FieldValue;
+
+/**
+ * Represents a filter that's filtering the metric {@link Record}s.
+ */
+@InterfaceAudience.Private
+public final class RecordFilter {
+
+ private enum Operator {
+EQUAL("="),
+DOUBLE_EQUALS("=="),
+GREATER(">"),
+GREATER_OR_EQUAL(">="),
+LESS("<"),
+LESS_OR_EQUAL("<=");
+
+private final String operator;
+
+Operator(String operator) {
+ this.operator = operator;
+}
+
+@Override
+public String toString() {
+ return operator;
+}
+ }
+
+ public static RecordFilter parse(String filterString, boolean ignoreCase) {
+return parse(filterString, Arrays.asList(Field.values()), ignoreCase);
+ }
+
+ public static RecordFilter parse(String filterString, List fields,
boolean ignoreCase) {
+int index = 0;
Review comment:
Okay sure.
Actually, at first, I was thinking of using ANTLR to parse filters but the
syntax of filters is very simple so I thought the lib was too much and decided
to use the simplest way (the current logic). Similarly, I think
`index-substring` way and `pattern-matching` way are too much in this case,
right? If we bring in a more complex syntax of filters to hbtop, we can
consider the better way. What do you think?
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] [hbase] brfrn169 commented on a change in pull request #647: HBASE-22988 Backport HBASE-11062 "hbtop" to branch-1
brfrn169 commented on a change in pull request #647: HBASE-22988 Backport
HBASE-11062 "hbtop" to branch-1
URL: https://github.com/apache/hbase/pull/647#discussion_r327041404
##
File path:
hbase-hbtop/src/main/java/org/apache/hadoop/hbase/hbtop/RecordFilter.java
##
@@ -0,0 +1,336 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.hadoop.hbase.hbtop;
+
+import java.util.Arrays;
+import java.util.List;
+import java.util.Objects;
+
+import org.apache.hadoop.hbase.classification.InterfaceAudience;
+import org.apache.hadoop.hbase.hbtop.field.Field;
+import org.apache.hadoop.hbase.hbtop.field.FieldValue;
+
+/**
+ * Represents a filter that's filtering the metric {@link Record}s.
+ */
+@InterfaceAudience.Private
+public final class RecordFilter {
+
+ private enum Operator {
+EQUAL("="),
+DOUBLE_EQUALS("=="),
+GREATER(">"),
+GREATER_OR_EQUAL(">="),
+LESS("<"),
+LESS_OR_EQUAL("<=");
+
+private final String operator;
+
+Operator(String operator) {
+ this.operator = operator;
+}
+
+@Override
+public String toString() {
+ return operator;
+}
+ }
+
+ public static RecordFilter parse(String filterString, boolean ignoreCase) {
+return parse(filterString, Arrays.asList(Field.values()), ignoreCase);
+ }
+
+ public static RecordFilter parse(String filterString, List fields,
boolean ignoreCase) {
+int index = 0;
Review comment:
Okay. What is the bad point in the code? Actually not sure that.
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] [hbase] brfrn169 commented on a change in pull request #647: HBASE-22988 Backport HBASE-11062 "hbtop" to branch-1
brfrn169 commented on a change in pull request #647: HBASE-22988 Backport
HBASE-11062 "hbtop" to branch-1
URL: https://github.com/apache/hbase/pull/647#discussion_r327016500
##
File path:
hbase-hbtop/src/main/java/org/apache/hadoop/hbase/hbtop/RecordFilter.java
##
@@ -0,0 +1,336 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.hadoop.hbase.hbtop;
+
+import java.util.Arrays;
+import java.util.List;
+import java.util.Objects;
+
+import org.apache.hadoop.hbase.classification.InterfaceAudience;
+import org.apache.hadoop.hbase.hbtop.field.Field;
+import org.apache.hadoop.hbase.hbtop.field.FieldValue;
+
+/**
+ * Represents a filter that's filtering the metric {@link Record}s.
+ */
+@InterfaceAudience.Private
+public final class RecordFilter {
+
+ private enum Operator {
+EQUAL("="),
+DOUBLE_EQUALS("=="),
+GREATER(">"),
+GREATER_OR_EQUAL(">="),
+LESS("<"),
+LESS_OR_EQUAL("<=");
+
+private final String operator;
+
+Operator(String operator) {
+ this.operator = operator;
+}
+
+@Override
+public String toString() {
+ return operator;
+}
+ }
+
+ public static RecordFilter parse(String filterString, boolean ignoreCase) {
+return parse(filterString, Arrays.asList(Field.values()), ignoreCase);
+ }
+
+ public static RecordFilter parse(String filterString, List fields,
boolean ignoreCase) {
+int index = 0;
Review comment:
Thanks.
> 1. A block of comments to make the purpose more clear, it's good to either
readability or maintainability.
Will do it.
> 2. Add a TODO in this method, need a better parsing way.
Do you have a better idea to parse filter strings? If so, could you please
share it with me? And I can rewrite the code for it.
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] [hbase] brfrn169 commented on a change in pull request #647: HBASE-22988 Backport HBASE-11062 "hbtop" to branch-1
brfrn169 commented on a change in pull request #647: HBASE-22988 Backport
HBASE-11062 "hbtop" to branch-1
URL: https://github.com/apache/hbase/pull/647#discussion_r327016500
##
File path:
hbase-hbtop/src/main/java/org/apache/hadoop/hbase/hbtop/RecordFilter.java
##
@@ -0,0 +1,336 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.hadoop.hbase.hbtop;
+
+import java.util.Arrays;
+import java.util.List;
+import java.util.Objects;
+
+import org.apache.hadoop.hbase.classification.InterfaceAudience;
+import org.apache.hadoop.hbase.hbtop.field.Field;
+import org.apache.hadoop.hbase.hbtop.field.FieldValue;
+
+/**
+ * Represents a filter that's filtering the metric {@link Record}s.
+ */
+@InterfaceAudience.Private
+public final class RecordFilter {
+
+ private enum Operator {
+EQUAL("="),
+DOUBLE_EQUALS("=="),
+GREATER(">"),
+GREATER_OR_EQUAL(">="),
+LESS("<"),
+LESS_OR_EQUAL("<=");
+
+private final String operator;
+
+Operator(String operator) {
+ this.operator = operator;
+}
+
+@Override
+public String toString() {
+ return operator;
+}
+ }
+
+ public static RecordFilter parse(String filterString, boolean ignoreCase) {
+return parse(filterString, Arrays.asList(Field.values()), ignoreCase);
+ }
+
+ public static RecordFilter parse(String filterString, List fields,
boolean ignoreCase) {
+int index = 0;
Review comment:
Thanks.
> 1. A block of comments to make the purpose more clear, it's good to either
readability or maintainability.
Will do it.
> 2. Add a TODO in this method, need a better parsing way.
Do you have a better idea to parse filter strings? If so, could you please
share it with me? And I will rewrite the code for it.
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] [hbase] brfrn169 commented on a change in pull request #647: HBASE-22988 Backport HBASE-11062 "hbtop" to branch-1
brfrn169 commented on a change in pull request #647: HBASE-22988 Backport HBASE-11062 "hbtop" to branch-1 URL: https://github.com/apache/hbase/pull/647#discussion_r327011908 ## File path: hbase-client/src/main/java/org/apache/hadoop/hbase/Size.java ## @@ -0,0 +1,157 @@ +/** + * Copyright The Apache Software Foundation + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * http://www.apache.org/licenses/LICENSE-2.0 + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package org.apache.hadoop.hbase; + +import java.math.BigDecimal; +import java.util.Objects; + +import org.apache.hadoop.hbase.classification.InterfaceAudience; + +/** + * It is used to represent the size with different units. + * This class doesn't serve for the precise computation. + */ +@InterfaceAudience.Public Review comment: Actually, the `Size` class is used by other classes in master & branch-2s and I just reused the class in the hbtop module. So we don't need to move the class to anywhere in master & branch-2s. In branch-1, we don't have the class so we needed to port the class to branch-1. This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [hbase] brfrn169 commented on a change in pull request #647: HBASE-22988 Backport HBASE-11062 "hbtop" to branch-1
brfrn169 commented on a change in pull request #647: HBASE-22988 Backport HBASE-11062 "hbtop" to branch-1 URL: https://github.com/apache/hbase/pull/647#discussion_r326611795 ## File path: hbase-client/src/main/java/org/apache/hadoop/hbase/Size.java ## @@ -0,0 +1,157 @@ +/** + * Copyright The Apache Software Foundation + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * http://www.apache.org/licenses/LICENSE-2.0 + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package org.apache.hadoop.hbase; + +import java.math.BigDecimal; +import java.util.Objects; + +import org.apache.hadoop.hbase.classification.InterfaceAudience; + +/** + * It is used to represent the size with different units. + * This class doesn't serve for the precise computation. + */ +@InterfaceAudience.Public Review comment: Done. This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [hbase] brfrn169 commented on a change in pull request #647: HBASE-22988 Backport HBASE-11062 "hbtop" to branch-1
brfrn169 commented on a change in pull request #647: HBASE-22988 Backport HBASE-11062 "hbtop" to branch-1 URL: https://github.com/apache/hbase/pull/647#discussion_r326608445 ## File path: hbase-client/src/main/java/org/apache/hadoop/hbase/Size.java ## @@ -0,0 +1,157 @@ +/** + * Copyright The Apache Software Foundation + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * http://www.apache.org/licenses/LICENSE-2.0 + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package org.apache.hadoop.hbase; + +import java.math.BigDecimal; +import java.util.Objects; + +import org.apache.hadoop.hbase.classification.InterfaceAudience; + +/** + * It is used to represent the size with different units. + * This class doesn't serve for the precise computation. + */ +@InterfaceAudience.Public Review comment: Maybe we can move it to the hbase-hbtop module and change to `@InterfaceAudience.Private`. Will do that. This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [hbase] brfrn169 commented on a change in pull request #647: HBASE-22988 Backport HBASE-11062 "hbtop" to branch-1
brfrn169 commented on a change in pull request #647: HBASE-22988 Backport HBASE-11062 "hbtop" to branch-1 URL: https://github.com/apache/hbase/pull/647#discussion_r326603252 ## File path: hbase-client/src/main/java/org/apache/hadoop/hbase/Size.java ## @@ -0,0 +1,157 @@ +/** + * Copyright The Apache Software Foundation + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * http://www.apache.org/licenses/LICENSE-2.0 + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package org.apache.hadoop.hbase; + +import java.math.BigDecimal; +import java.util.Objects; + +import org.apache.hadoop.hbase.classification.InterfaceAudience; + +/** + * It is used to represent the size with different units. + * This class doesn't serve for the precise computation. + */ +@InterfaceAudience.Public Review comment: It's a backport from the master branch actually and it's `@InterfaceAudience.Public` there. Should we change it to `@InterfaceAudience.Private` in branch-1? This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [hbase] brfrn169 commented on a change in pull request #647: HBASE-22988 Backport HBASE-11062 "hbtop" to branch-1
brfrn169 commented on a change in pull request #647: HBASE-22988 Backport HBASE-11062 "hbtop" to branch-1 URL: https://github.com/apache/hbase/pull/647#discussion_r326602033 ## File path: hbase-hbtop/src/main/java/org/apache/hadoop/hbase/hbtop/HBTop.java ## @@ -0,0 +1,140 @@ +/** + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package org.apache.hadoop.hbase.hbtop; + +import java.util.Objects; + +import org.apache.commons.cli.BasicParser; +import org.apache.commons.cli.CommandLine; +import org.apache.commons.cli.HelpFormatter; +import org.apache.commons.cli.Options; +import org.apache.hadoop.conf.Configuration; +import org.apache.hadoop.conf.Configured; +import org.apache.hadoop.hbase.HBaseConfiguration; +import org.apache.hadoop.hbase.HBaseInterfaceAudience; +import org.apache.hadoop.hbase.classification.InterfaceAudience; +import org.apache.hadoop.hbase.hbtop.mode.Mode; +import org.apache.hadoop.hbase.hbtop.screen.Screen; +import org.apache.hadoop.util.Tool; +import org.apache.hadoop.util.ToolRunner; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; + Review comment: Changed to use commons-logging. Thanks. This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [hbase] brfrn169 commented on a change in pull request #647: HBASE-22988 Backport HBASE-11062 "hbtop" to branch-1
brfrn169 commented on a change in pull request #647: HBASE-22988 Backport
HBASE-11062 "hbtop" to branch-1
URL: https://github.com/apache/hbase/pull/647#discussion_r326601774
##
File path: conf/log4j-hbtop.properties
##
@@ -0,0 +1,27 @@
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements. See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership. The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+
+log4j.rootLogger=WARN,console
+log4j.threshold=WARN
+
+# console
+log4j.appender.console=org.apache.log4j.ConsoleAppender
+log4j.appender.console.target=System.err
+log4j.appender.console.layout=org.apache.log4j.PatternLayout
+log4j.appender.console.layout.ConversionPattern=%d{ISO8601} %-5p [%t] %c{2}:
%m%n
+
+# ZooKeeper will still put stuff at WARN
+log4j.logger.org.apache.zookeeper=ERROR
Review comment:
Fixed. Thanks.
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
