You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@hbase.apache.org by GitBox <gi...@apache.org> on 2020/07/08 23:41:16 UTC
[GitHub] [hbase] skochhar commented on a change in pull request #1935: HBASE-22146 SpaceQuotaViolationPolicy Disable is not working in Names…
skochhar commented on a change in pull request #1935:
URL: https://github.com/apache/hbase/pull/1935#discussion_r447974191
##########
File path: hbase-server/src/main/java/org/apache/hadoop/hbase/namespace/NamespaceAuditor.java
##########
@@ -18,6 +18,7 @@
package org.apache.hadoop.hbase.namespace;
import java.io.IOException;
+import java.util.Set;
Review comment:
Good catch Sakthi, I will remove the import
##########
File path: hbase-server/src/main/java/org/apache/hadoop/hbase/quotas/QuotaUtil.java
##########
@@ -264,17 +265,33 @@ private static void deleteQuotas(final Connection connection, final byte[] rowKe
final byte[] qualifier) throws IOException {
Delete delete = new Delete(rowKey);
if (qualifier != null) {
- delete.addColumns(QUOTA_FAMILY_INFO, qualifier);
+ if (Arrays.equals(qualifier,QUOTA_QUALIFIER_POLICY)) {
+ delete.addColumns(QUOTA_FAMILY_USAGE, qualifier);
+ } else
+ delete.addColumns(QUOTA_FAMILY_INFO, qualifier);
}
- doDelete(connection, delete);
if (isNamespaceRowKey(rowKey)) {
- TableName[] tableArray = connection.getAdmin().listTableNamesByNamespace(getNamespaceFromRowKey(rowKey));
- for (TableName tableName: tableArray) {
- if (QuotaUtil.getTableQuota(connection, tableName) == null) {
- deleteTableQuota(connection,tableName);
+ //Check namespace is not deleted before you get info about quota and list of tables in namespace
+ NamespaceDescriptor[] descs = connection.getAdmin().listNamespaceDescriptors();
+ String ns = getNamespaceFromRowKey(rowKey);
+ int index = 0;
+ while (index < descs.length) {
+ if (ns.equals(descs[index].getName())) {
+ Quotas namespaceQuota = getNamespaceQuota(connection,ns);
+ if (namespaceQuota != null && namespaceQuota.hasSpace()) {
+ TableName[] tableArray = connection.getAdmin().listTableNamesByNamespace(ns);
+ for (TableName tableName : tableArray) {
+ deleteQuotas(connection, getTableRowKey(tableName), QUOTA_QUALIFIER_POLICY);
+ }
+ }
+ //Exit the while loop by moving to last index
+ index = descs.length;
+ } else {
+ index++;
}
}
}
+ doDelete(connection, delete);
Review comment:
> What would happen if we get an exception trying to delete a quota? Would it be necessary to retry the operation again?
Thanks for reviewing Esteban.
I am doing null checks everywhere to avoid NPEs. I tested this code with a namespace quota on a namespace both without any tables, and with tables and the code worked fine.
I see that in line 282 connection.getAdmin().listTableNamesByNamespace(ns) can throw IOException. I can add try-catch around my new code to avoid the final doDelete() for the namespace getting impacted from any exception in the new code.
##########
File path: hbase-server/src/main/java/org/apache/hadoop/hbase/quotas/QuotaUtil.java
##########
@@ -264,17 +265,33 @@ private static void deleteQuotas(final Connection connection, final byte[] rowKe
final byte[] qualifier) throws IOException {
Delete delete = new Delete(rowKey);
if (qualifier != null) {
- delete.addColumns(QUOTA_FAMILY_INFO, qualifier);
+ if (Arrays.equals(qualifier,QUOTA_QUALIFIER_POLICY)) {
+ delete.addColumns(QUOTA_FAMILY_USAGE, qualifier);
+ } else
+ delete.addColumns(QUOTA_FAMILY_INFO, qualifier);
}
- doDelete(connection, delete);
if (isNamespaceRowKey(rowKey)) {
- TableName[] tableArray = connection.getAdmin().listTableNamesByNamespace(getNamespaceFromRowKey(rowKey));
- for (TableName tableName: tableArray) {
- if (QuotaUtil.getTableQuota(connection, tableName) == null) {
- deleteTableQuota(connection,tableName);
+ //Check namespace is not deleted before you get info about quota and list of tables in namespace
+ NamespaceDescriptor[] descs = connection.getAdmin().listNamespaceDescriptors();
+ String ns = getNamespaceFromRowKey(rowKey);
+ int index = 0;
+ while (index < descs.length) {
+ if (ns.equals(descs[index].getName())) {
+ Quotas namespaceQuota = getNamespaceQuota(connection,ns);
+ if (namespaceQuota != null && namespaceQuota.hasSpace()) {
+ TableName[] tableArray = connection.getAdmin().listTableNamesByNamespace(ns);
+ for (TableName tableName : tableArray) {
+ deleteQuotas(connection, getTableRowKey(tableName), QUOTA_QUALIFIER_POLICY);
+ }
+ }
+ //Exit the while loop by moving to last index
+ index = descs.length;
+ } else {
+ index++;
}
}
}
+ doDelete(connection, delete);
Review comment:
I checked more usages of this method and other files where getAdmin().listTableNamesByNamespace(ns) is used and in case of error they let the calling method handle it and allow the user to decide to retry the operation.
The call for deleteQuotas() also mentions about throwing IOException like other methods:
private static void deleteQuotas(final Connection connection, final byte[] rowKey,
final byte[] qualifier) throws IOException
##########
File path: hbase-server/src/main/java/org/apache/hadoop/hbase/namespace/NamespaceAuditor.java
##########
@@ -18,6 +18,7 @@
package org.apache.hadoop.hbase.namespace;
import java.io.IOException;
+import java.util.Set;
Review comment:
Added a new PR where I removed the import
----------------------------------------------------------------
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:
users@infra.apache.org