Skip to content

Log ZooKeeper-backed property changes #6465

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
DomGarguilo wants to merge 1 commit into
base: main
Choose a base branch
from
+57 −0
Open

Log ZooKeeper-backed property changes #6465

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
57 changes: 57 additions & 0 deletions server/manager/src/main/java/org/apache/accumulo/manager/ManagerClientServiceHandler.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,7 @@
import java.util.Map.Entry;
import java.util.Optional;
import java.util.Set;
import java.util.TreeMap;
import java.util.concurrent.CompletableFuture;
import java.util.concurrent.TimeUnit;

Expand Down Expand Up @@ -104,13 +105,16 @@
import org.apache.zookeeper.KeeperException;
import org.apache.zookeeper.KeeperException.NoNodeException;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;

import com.google.common.base.Preconditions;
import com.google.common.collect.Lists;

public class ManagerClientServiceHandler implements ManagerClientService.Iface {

private static final Logger log = Manager.log;
private static final Logger configLog =
LoggerFactory.getLogger("org.apache.accumulo.configuration");
private final Manager manager;
private final ServerContext context;
private final AuditedSecurityOperation security;
Expand Down Expand Up @@ -279,6 +283,8 @@ public void modifyTableProperties(TInfo tinfo, TCredentials credentials, String
.getAllPropertiesWithPrefix(Property.TABLE_ITERATOR_PREFIX));
PropUtil.replaceProperties(context, TablePropKey.of(tableId), properties.getVersion(),
properties.getProperties());
logModifyProperties(credentials, "table", tableName, properties.getVersion(),
properties.getProperties());
} catch (ConcurrentModificationException cme) {
log.warn("Error modifying table properties, properties have changed", cme);
throw new ThriftConcurrentModificationException(cme.getMessage());
Expand Down Expand Up @@ -419,6 +425,7 @@ public void removeSystemProperty(TInfo info, TCredentials c, String property)

try {
SystemPropUtil.removeSystemProperty(context, property);
logRemoveProperty(c, "system", "system", property);
} catch (Exception e) {
Manager.log.error("Problem removing config property in zookeeper", e);
throw new RuntimeException(e.getMessage());
Expand All @@ -434,6 +441,7 @@ public void setSystemProperty(TInfo info, TCredentials c, String property, Strin

try {
SystemPropUtil.setSystemProperty(context, property, value);
logSetProperty(c, "system", "system", property, value);
} catch (IllegalArgumentException iae) {
Manager.log.error("Problem setting invalid property", iae);
throw new ThriftPropertyException(property, value,
Expand All @@ -453,6 +461,8 @@ public void modifySystemProperties(TInfo info, TCredentials c, TVersionedPropert

try {
SystemPropUtil.modifyProperties(context, properties.getVersion(), properties.getProperties());
logModifyProperties(c, "system", "system", properties.getVersion(),
properties.getProperties());
} catch (IllegalArgumentException iae) {
Manager.log.error("Problem setting invalid property", iae);
throw new ThriftPropertyException("Modify properties", "failed", iae.getMessage());
Expand Down Expand Up @@ -521,6 +531,7 @@ public void removeResourceGroupProperty(TInfo info, TCredentials c, String resou
ResourceGroupId rgid = ClientServiceHandler.checkResourceGroupId(context, resourceGroup);
try {
PropUtil.removeProperties(context, ResourceGroupPropKey.of(rgid), List.of(property));
logRemoveProperty(c, "resourceGroup", resourceGroup, property);
} catch (Exception e) {
Manager.log.error("Problem removing config property in zookeeper", e);
throw new TException(e.getMessage());
Expand All @@ -537,6 +548,7 @@ public void setResourceGroupProperty(TInfo info, TCredentials c, String resource
final ResourceGroupId rgid = ClientServiceHandler.checkResourceGroupId(context, resourceGroup);
try {
PropUtil.setProperties(context, ResourceGroupPropKey.of(rgid), Map.of(property, value));
logSetProperty(c, "resourceGroup", resourceGroup, property, value);
} catch (IllegalArgumentException iae) {
Manager.log.error("Problem setting invalid property", iae);
throw new ThriftPropertyException(property, value,
Expand All @@ -557,6 +569,8 @@ public void modifyResourceGroupProperties(TInfo info, TCredentials c, String res
try {
PropUtil.replaceProperties(context, ResourceGroupPropKey.of(rgid), properties.getVersion(),
properties.getProperties());
logModifyProperties(c, "resourceGroup", resourceGroup, properties.getVersion(),
properties.getProperties());
} catch (IllegalArgumentException iae) {
Manager.log.error("Problem setting invalid property", iae);
throw new ThriftPropertyException("Modify properties", "failed", iae.getMessage());
Expand Down Expand Up @@ -592,6 +606,8 @@ public void modifyNamespaceProperties(TInfo tinfo, TCredentials credentials, Str
context.getConfiguration().getAllPropertiesWithPrefix(Property.TABLE_ITERATOR_PREFIX));
PropUtil.replaceProperties(context, NamespacePropKey.of(namespaceId), properties.getVersion(),
properties.getProperties());
logModifyProperties(credentials, "namespace", ns, properties.getVersion(),
properties.getProperties());
} catch (ConcurrentModificationException cme) {
log.warn("Error modifying namespace properties, properties have changed", cme);
throw new ThriftConcurrentModificationException(cme.getMessage());
Expand Down Expand Up @@ -628,11 +644,13 @@ private void alterNamespaceProperty(TCredentials c, String namespace, String pro
try {
if (value == null) {
PropUtil.removeProperties(context, NamespacePropKey.of(namespaceId), List.of(property));
logRemoveProperty(c, "namespace", namespace, property);
} else {
checkIteratorPriorityConflicts("namespace:" + namespace + " namespaceId:" + namespaceId,
Map.of(property, value), context.getNamespaceConfiguration(namespaceId)
.getAllPropertiesWithPrefix(Property.TABLE_ITERATOR_PREFIX));
PropUtil.setProperties(context, NamespacePropKey.of(namespaceId), Map.of(property, value));
logSetProperty(c, "namespace", namespace, property, value);
}
} catch (IllegalStateException ex) {
// race condition on delete... namespace no longer exists? An undelying ZooKeeper.NoNode
Expand All @@ -658,6 +676,7 @@ private void alterTableProperty(TCredentials c, String tableName, String propert
try {
if (op == TableOperation.REMOVE_PROPERTY) {
PropUtil.removeProperties(context, TablePropKey.of(tableId), List.of(property));
logRemoveProperty(c, "table", tableName, property);
} else if (op == TableOperation.SET_PROPERTY) {
if (value == null || value.isEmpty()) {
value = "";
Expand All @@ -666,6 +685,7 @@ private void alterTableProperty(TCredentials c, String tableName, String propert
Map.of(property, value), context.getTableConfiguration(tableId)
.getAllPropertiesWithPrefix(Property.TABLE_ITERATOR_PREFIX));
PropUtil.setProperties(context, TablePropKey.of(tableId), Map.of(property, value));
logSetProperty(c, "table", tableName, property, value);
} else {
throw new UnsupportedOperationException("table operation:" + op.name());
}
Expand All @@ -683,6 +703,43 @@ private void alterTableProperty(TCredentials c, String tableName, String propert
}
}

private static void logSetProperty(TCredentials credentials, String scope, String target,
String property, String value) {
if (!configLog.isInfoEnabled()) {
return;
}
configLog.info("action=set; user={}; scope={}; target={}; property={}; value={};",
credentials.getPrincipal(), scope, target, property, printableValue(property, value));
}

private static void logRemoveProperty(TCredentials credentials, String scope, String target,
String property) {
if (!configLog.isInfoEnabled()) {
return;
}
configLog.info("action=remove; user={}; scope={}; target={}; property={};",
credentials.getPrincipal(), scope, target, property);
}

private static void logModifyProperties(TCredentials credentials, String scope, String target,
long version, Map<String,String> properties) {
if (!configLog.isInfoEnabled()) {
return;
}
configLog.info("action=modify; user={}; scope={}; target={}; version={}; properties={};",
credentials.getPrincipal(), scope, target, version, printableProperties(properties));
}

private static Map<String,String> printableProperties(Map<String,String> properties) {
Map<String,String> printable = new TreeMap<>();
properties.forEach((key, value) -> printable.put(key, printableValue(key, value)));
return printable;
}

private static String printableValue(String property, String value) {
return Property.isSensitive(property) ? "<hidden>" : value;
}

@Override
public void waitForBalance(TInfo tinfo) {
manager.getBalanceManager().waitForBalance();
Expand Down