anoopsjohn commented on a change in pull request #1681: URL: https://github.com/apache/hbase/pull/1681#discussion_r425925567
########## File path: hbase-server/src/main/java/org/apache/hadoop/hbase/master/slowlog/SlowLogMasterService.java ########## @@ -0,0 +1,72 @@ +/* + * + * 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.master.slowlog; + +import java.io.IOException; +import org.apache.hadoop.conf.Configuration; +import org.apache.hadoop.hbase.HConstants; +import org.apache.hadoop.hbase.MetaTableAccessor; +import org.apache.hadoop.hbase.TableName; +import org.apache.hadoop.hbase.client.ColumnFamilyDescriptorBuilder; +import org.apache.hadoop.hbase.client.TableDescriptorBuilder; +import org.apache.hadoop.hbase.master.MasterServices; +import org.apache.yetus.audience.InterfaceAudience; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; + +/** + * Slowlog Master services - Table creation to be used by HMaster + */ [email protected] +public class SlowLogMasterService { + + private static final Logger LOG = LoggerFactory.getLogger(SlowLogMasterService.class); + + private final boolean slowlogTableEnabled; + private final MasterServices masterServices; + + private static final TableDescriptorBuilder TABLE_DESCRIPTOR_BUILDER = + TableDescriptorBuilder.newBuilder(TableName.SLOW_LOG_TABLE_NAME).setRegionReplication(1) + .setColumnFamily( + ColumnFamilyDescriptorBuilder.newBuilder(HConstants.SLOWLOG_INFO_FAMILY) + .setScope(HConstants.REPLICATION_SCOPE_LOCAL) + .setInMemory(true) Review comment: We should make the blockCaching = false for this table? ########## File path: hbase-server/src/main/java/org/apache/hadoop/hbase/master/slowlog/SlowLogMasterService.java ########## @@ -0,0 +1,72 @@ +/* + * + * 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.master.slowlog; + +import java.io.IOException; +import org.apache.hadoop.conf.Configuration; +import org.apache.hadoop.hbase.HConstants; +import org.apache.hadoop.hbase.MetaTableAccessor; +import org.apache.hadoop.hbase.TableName; +import org.apache.hadoop.hbase.client.ColumnFamilyDescriptorBuilder; +import org.apache.hadoop.hbase.client.TableDescriptorBuilder; +import org.apache.hadoop.hbase.master.MasterServices; +import org.apache.yetus.audience.InterfaceAudience; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; + +/** + * Slowlog Master services - Table creation to be used by HMaster + */ [email protected] +public class SlowLogMasterService { + + private static final Logger LOG = LoggerFactory.getLogger(SlowLogMasterService.class); + + private final boolean slowlogTableEnabled; + private final MasterServices masterServices; + + private static final TableDescriptorBuilder TABLE_DESCRIPTOR_BUILDER = + TableDescriptorBuilder.newBuilder(TableName.SLOW_LOG_TABLE_NAME).setRegionReplication(1) + .setColumnFamily( + ColumnFamilyDescriptorBuilder.newBuilder(HConstants.SLOWLOG_INFO_FAMILY) + .setScope(HConstants.REPLICATION_SCOPE_LOCAL) + .setInMemory(true) Review comment: Why we need inMemory true ? ########## File path: hbase-common/src/main/java/org/apache/hadoop/hbase/TableName.java ########## @@ -83,6 +83,12 @@ public static final TableName META_TABLE_NAME = valueOf(NamespaceDescriptor.SYSTEM_NAMESPACE_NAME_STR, "meta"); + /** hbase:slowlog table name - can be enabled + * with config - hbase.regionserver.slowlog.systable.enabled + */ + public static final TableName SLOW_LOG_TABLE_NAME = Review comment: We are exposing this Table name for users right? I think we need. Then we should expose the cf and column names also? Sure this is not the class for those expose. Now we allow all kind of client ops (writes, disable, alter) on this table? I believe in Master branch we allow tables in system NS to be disabled, altered etc. There was a Jira around META table. Will that be ok (Disable this table and keep it that state) ########## File path: hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/slowlog/SlowLogTableAccessor.java ########## @@ -0,0 +1,124 @@ +/* + * + * 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.regionserver.slowlog; + +import java.io.IOException; +import java.util.ArrayList; +import java.util.List; +import java.util.Random; +import org.apache.hadoop.hbase.HConstants; +import org.apache.hadoop.hbase.TableName; +import org.apache.hadoop.hbase.client.Connection; +import org.apache.hadoop.hbase.client.Durability; +import org.apache.hadoop.hbase.client.Put; +import org.apache.hadoop.hbase.client.Table; +import org.apache.hadoop.hbase.shaded.protobuf.generated.TooSlowLog; +import org.apache.hadoop.hbase.util.Bytes; +import org.apache.hadoop.hbase.util.EnvironmentEdgeManager; +import org.apache.yetus.audience.InterfaceAudience; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; + +/** + * Slowlog Accessor to record slow/large RPC log identified at each RegionServer RpcServer level. + * This can be done only optionally to record the entire history of slow/large rpc calls + * since RingBuffer can handle only limited latest records. + */ [email protected] +public class SlowLogTableAccessor { + + private static final Logger LOG = LoggerFactory.getLogger(SlowLogTableAccessor.class); + + private static final Random RANDOM = new Random(); + + private static void doPut(final Connection connection, final List<Put> puts) + throws IOException { + try (Table table = connection.getTable(TableName.SLOW_LOG_TABLE_NAME)) { + table.put(puts); + } + } + + /** + * Add slow/large log records to hbase:slowlog table + * + * @param slowLogPayloads List of SlowLogPayload to process + * @param connection Connection to put data + */ + public static void addSlowLogRecords(final List<TooSlowLog.SlowLogPayload> slowLogPayloads, + final Connection connection) { + List<Put> puts = new ArrayList<>(); Review comment: Puts list size can be given as slowLogPayloads.size() ########## File path: hbase-server/src/main/java/org/apache/hadoop/hbase/master/slowlog/SlowLogMasterService.java ########## @@ -0,0 +1,72 @@ +/* + * + * 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.master.slowlog; + +import java.io.IOException; +import org.apache.hadoop.conf.Configuration; +import org.apache.hadoop.hbase.HConstants; +import org.apache.hadoop.hbase.MetaTableAccessor; +import org.apache.hadoop.hbase.TableName; +import org.apache.hadoop.hbase.client.ColumnFamilyDescriptorBuilder; +import org.apache.hadoop.hbase.client.TableDescriptorBuilder; +import org.apache.hadoop.hbase.master.MasterServices; +import org.apache.yetus.audience.InterfaceAudience; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; + +/** + * Slowlog Master services - Table creation to be used by HMaster + */ [email protected] +public class SlowLogMasterService { + + private static final Logger LOG = LoggerFactory.getLogger(SlowLogMasterService.class); + + private final boolean slowlogTableEnabled; + private final MasterServices masterServices; + + private static final TableDescriptorBuilder TABLE_DESCRIPTOR_BUILDER = + TableDescriptorBuilder.newBuilder(TableName.SLOW_LOG_TABLE_NAME).setRegionReplication(1) + .setColumnFamily( + ColumnFamilyDescriptorBuilder.newBuilder(HConstants.SLOWLOG_INFO_FAMILY) + .setScope(HConstants.REPLICATION_SCOPE_LOCAL) + .setInMemory(true) + .setMaxVersions(1).build()); + + public SlowLogMasterService(final Configuration configuration, + final MasterServices masterServices) { + slowlogTableEnabled = configuration.getBoolean(HConstants.SLOW_LOG_SYS_TABLE_ENABLED_KEY, + HConstants.DEFAULT_SLOW_LOG_SYS_TABLE_ENABLED_KEY); + this.masterServices = masterServices; + } + + public void init() throws IOException { + if (!slowlogTableEnabled) { + LOG.info("SlowLogTable is not enabled. Quitting."); Review comment: The log is a bit confusing. The will look like a table is enabled/disabled. We can say slow request logging to system table is not enable? (Or a better message) ########## File path: hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/slowlog/LogEventHandler.java ########## @@ -256,4 +278,32 @@ private boolean isFilterProvided(AdminProtos.SlowLogResponseRequest request) { return filteredSlowLogPayloads; } + /** + * Poll from queueForSysTable and insert 100 records in hbase:slowlog table in single batch + */ + void addAllLogsToSysTable() { + if (queueForSysTable == null) { + LOG.warn("hbase.regionserver.slowlog.systable.enabled is turned off. Exiting."); Review comment: If the config is turned off, this will log this warn line in every 10 mins right? Dont think we need to log here at all. ########## File path: hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/slowlog/LogEventHandler.java ########## @@ -256,4 +278,32 @@ private boolean isFilterProvided(AdminProtos.SlowLogResponseRequest request) { return filteredSlowLogPayloads; } + /** + * Poll from queueForSysTable and insert 100 records in hbase:slowlog table in single batch + */ + void addAllLogsToSysTable() { + if (queueForSysTable == null) { + LOG.warn("hbase.regionserver.slowlog.systable.enabled is turned off. Exiting."); + return; + } + if (this.connection == null) { + LOG.warn("LogEventHandler has null connection. Exiting."); + return; + } + List<SlowLogPayload> slowLogPayloads = new ArrayList<>(); + int i = 0; + while (!queueForSysTable.isEmpty()) { + slowLogPayloads.add(queueForSysTable.poll()); + i++; + if (i == 100) { + SlowLogTableAccessor.addSlowLogRecords(slowLogPayloads, this.connection); + slowLogPayloads = new ArrayList<>(); Review comment: Instead can do ArrayList#clear? ########## File path: hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/slowlog/LogEventHandler.java ########## @@ -256,4 +278,32 @@ private boolean isFilterProvided(AdminProtos.SlowLogResponseRequest request) { return filteredSlowLogPayloads; } + /** + * Poll from queueForSysTable and insert 100 records in hbase:slowlog table in single batch + */ + void addAllLogsToSysTable() { + if (queueForSysTable == null) { + LOG.warn("hbase.regionserver.slowlog.systable.enabled is turned off. Exiting."); + return; + } + if (this.connection == null) { + LOG.warn("LogEventHandler has null connection. Exiting."); + return; + } + List<SlowLogPayload> slowLogPayloads = new ArrayList<>(); + int i = 0; + while (!queueForSysTable.isEmpty()) { + slowLogPayloads.add(queueForSysTable.poll()); + i++; + if (i == 100) { Review comment: If 100 rows, approx what will be the write req total size? ########## File path: hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/slowlog/SlowLogTableAccessor.java ########## @@ -0,0 +1,124 @@ +/* + * + * 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.regionserver.slowlog; + +import java.io.IOException; +import java.util.ArrayList; +import java.util.List; +import java.util.Random; +import org.apache.hadoop.hbase.HConstants; +import org.apache.hadoop.hbase.TableName; +import org.apache.hadoop.hbase.client.Connection; +import org.apache.hadoop.hbase.client.Durability; +import org.apache.hadoop.hbase.client.Put; +import org.apache.hadoop.hbase.client.Table; +import org.apache.hadoop.hbase.shaded.protobuf.generated.TooSlowLog; +import org.apache.hadoop.hbase.util.Bytes; +import org.apache.hadoop.hbase.util.EnvironmentEdgeManager; +import org.apache.yetus.audience.InterfaceAudience; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; + +/** + * Slowlog Accessor to record slow/large RPC log identified at each RegionServer RpcServer level. + * This can be done only optionally to record the entire history of slow/large rpc calls + * since RingBuffer can handle only limited latest records. + */ [email protected] +public class SlowLogTableAccessor { + + private static final Logger LOG = LoggerFactory.getLogger(SlowLogTableAccessor.class); + + private static final Random RANDOM = new Random(); + + private static void doPut(final Connection connection, final List<Put> puts) + throws IOException { + try (Table table = connection.getTable(TableName.SLOW_LOG_TABLE_NAME)) { + table.put(puts); Review comment: For some reason this put is not happening for one occasion, we will just throw away those logs. Anyways we are ok to loose some data as we already disabled the WAL. ########## File path: hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/slowlog/SlowLogTableAccessor.java ########## @@ -0,0 +1,124 @@ +/* + * + * 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.regionserver.slowlog; + +import java.io.IOException; +import java.util.ArrayList; +import java.util.List; +import java.util.Random; +import org.apache.hadoop.hbase.HConstants; +import org.apache.hadoop.hbase.TableName; +import org.apache.hadoop.hbase.client.Connection; +import org.apache.hadoop.hbase.client.Durability; +import org.apache.hadoop.hbase.client.Put; +import org.apache.hadoop.hbase.client.Table; +import org.apache.hadoop.hbase.shaded.protobuf.generated.TooSlowLog; +import org.apache.hadoop.hbase.util.Bytes; +import org.apache.hadoop.hbase.util.EnvironmentEdgeManager; +import org.apache.yetus.audience.InterfaceAudience; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; + +/** + * Slowlog Accessor to record slow/large RPC log identified at each RegionServer RpcServer level. + * This can be done only optionally to record the entire history of slow/large rpc calls + * since RingBuffer can handle only limited latest records. + */ [email protected] +public class SlowLogTableAccessor { + + private static final Logger LOG = LoggerFactory.getLogger(SlowLogTableAccessor.class); + + private static final Random RANDOM = new Random(); + + private static void doPut(final Connection connection, final List<Put> puts) + throws IOException { + try (Table table = connection.getTable(TableName.SLOW_LOG_TABLE_NAME)) { + table.put(puts); Review comment: By default HBase will have a 60 sec RPC timeout and 35 retries. Means overall it will be > 10 min which is the default Chore interval. We can adjust these configs? 35 retries anyways we can reduce. ########## File path: hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/slowlog/SlowLogTableAccessor.java ########## @@ -0,0 +1,124 @@ +/* + * + * 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.regionserver.slowlog; + +import java.io.IOException; +import java.util.ArrayList; +import java.util.List; +import java.util.Random; +import org.apache.hadoop.hbase.HConstants; +import org.apache.hadoop.hbase.TableName; +import org.apache.hadoop.hbase.client.Connection; +import org.apache.hadoop.hbase.client.Durability; +import org.apache.hadoop.hbase.client.Put; +import org.apache.hadoop.hbase.client.Table; +import org.apache.hadoop.hbase.shaded.protobuf.generated.TooSlowLog; +import org.apache.hadoop.hbase.util.Bytes; +import org.apache.hadoop.hbase.util.EnvironmentEdgeManager; +import org.apache.yetus.audience.InterfaceAudience; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; + +/** + * Slowlog Accessor to record slow/large RPC log identified at each RegionServer RpcServer level. + * This can be done only optionally to record the entire history of slow/large rpc calls + * since RingBuffer can handle only limited latest records. + */ [email protected] +public class SlowLogTableAccessor { + + private static final Logger LOG = LoggerFactory.getLogger(SlowLogTableAccessor.class); + + private static final Random RANDOM = new Random(); + + private static void doPut(final Connection connection, final List<Put> puts) + throws IOException { + try (Table table = connection.getTable(TableName.SLOW_LOG_TABLE_NAME)) { + table.put(puts); + } + } + + /** + * Add slow/large log records to hbase:slowlog table + * + * @param slowLogPayloads List of SlowLogPayload to process + * @param connection Connection to put data + */ + public static void addSlowLogRecords(final List<TooSlowLog.SlowLogPayload> slowLogPayloads, + final Connection connection) { + List<Put> puts = new ArrayList<>(); + for (TooSlowLog.SlowLogPayload slowLogPayload : slowLogPayloads) { + final byte[] rowKey = getRowKey(slowLogPayload); + final Put put = new Put(rowKey).setDurability(Durability.SKIP_WAL) + .addColumn(HConstants.SLOWLOG_INFO_FAMILY, Bytes.toBytes("call_details"), + Bytes.toBytes(slowLogPayload.getCallDetails())) + .addColumn(HConstants.SLOWLOG_INFO_FAMILY, Bytes.toBytes("client_address"), + Bytes.toBytes(slowLogPayload.getClientAddress())) + .addColumn(HConstants.SLOWLOG_INFO_FAMILY, Bytes.toBytes("method_name"), + Bytes.toBytes(slowLogPayload.getMethodName())) + .addColumn(HConstants.SLOWLOG_INFO_FAMILY, Bytes.toBytes("param"), + Bytes.toBytes(slowLogPayload.getParam())) + .addColumn(HConstants.SLOWLOG_INFO_FAMILY, Bytes.toBytes("processing_time"), + Bytes.toBytes(slowLogPayload.getProcessingTime())) + .addColumn(HConstants.SLOWLOG_INFO_FAMILY, Bytes.toBytes("queue_time"), + Bytes.toBytes(slowLogPayload.getQueueTime())) + .addColumn(HConstants.SLOWLOG_INFO_FAMILY, Bytes.toBytes("region_name"), + Bytes.toBytes(slowLogPayload.getRegionName())) + .addColumn(HConstants.SLOWLOG_INFO_FAMILY, Bytes.toBytes("response_size"), + Bytes.toBytes(slowLogPayload.getResponseSize())) + .addColumn(HConstants.SLOWLOG_INFO_FAMILY, Bytes.toBytes("server_class"), + Bytes.toBytes(slowLogPayload.getServerClass())) + .addColumn(HConstants.SLOWLOG_INFO_FAMILY, Bytes.toBytes("start_time"), + Bytes.toBytes(slowLogPayload.getStartTime())) + .addColumn(HConstants.SLOWLOG_INFO_FAMILY, Bytes.toBytes("type"), + Bytes.toBytes(slowLogPayload.getType().name())) + .addColumn(HConstants.SLOWLOG_INFO_FAMILY, Bytes.toBytes("username"), + Bytes.toBytes(slowLogPayload.getUserName())); + puts.add(put); + } + try { + doPut(connection, puts); + } catch (IOException e) { Review comment: Is there any chance of non IOE possible? Should we catch Exception. Else it might cause the chore to get terminated if any non IOE comes ever? ---------------------------------------------------------------- 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: [email protected]
