Skip to content

Commit

Permalink
Introduced procedure dbms.setConfigValue to allow config changes at r…
Browse files Browse the repository at this point in the history
…untime
  • Loading branch information
klaren committed Aug 15, 2017
1 parent b6b690b commit a18079f
Show file tree
Hide file tree
Showing 12 changed files with 356 additions and 35 deletions.
Expand Up @@ -60,7 +60,7 @@ public List<ConfigValue> asConfigValues( @Nonnull Map<String,String> validConfig
return new ConfigValue( setting.name(), setting.description(),
setting.documentedDefaultValue(),
Optional.ofNullable( val.getValue() ),
setting.valueDescription(), setting.internal(),
setting.valueDescription(), setting.internal(), setting.dynamic(),
setting.deprecated(), setting.replacement() );
} )
.collect( Collectors.toList() );
Expand Down
Expand Up @@ -34,12 +34,13 @@ public class ConfigValue
private final Optional<?> value;
private final String valueDescription;
private final boolean internal;
private final boolean dynamic;
private final boolean deprecated;
private final Optional<String> replacement;

public ConfigValue( @Nonnull String name, @Nonnull Optional<String> description,
@Nonnull Optional<String> documentedDefaultValue, @Nonnull Optional<?> value,
@Nonnull String valueDescription, boolean internal, boolean deprecated,
@Nonnull String valueDescription, boolean internal, boolean dynamic, boolean deprecated,
@Nonnull Optional<String> replacement )
{
this.name = name;
Expand All @@ -48,6 +49,7 @@ public ConfigValue( @Nonnull String name, @Nonnull Optional<String> description,
this.value = value;
this.valueDescription = valueDescription;
this.internal = internal;
this.dynamic = dynamic;
this.deprecated = deprecated;
this.replacement = replacement;
}
Expand Down Expand Up @@ -98,6 +100,11 @@ public boolean internal()
return internal;
}

public boolean dynamic()
{
return dynamic;
}

@Nonnull
public Optional<String> documentedDefaultValue()
{
Expand Down
Expand Up @@ -35,7 +35,7 @@ public class ConfigValueTest
public void handlesEmptyValue() throws Exception
{
ConfigValue value = new ConfigValue( "name", Optional.empty(), Optional.empty(), Optional.empty(),
"description", false, false, Optional.empty() );
"description", false, false, false, Optional.empty() );

assertEquals( Optional.empty(), value.value() );
assertEquals( "null", value.toString() );
Expand All @@ -48,7 +48,7 @@ public void handlesEmptyValue() throws Exception
public void handlesInternal() throws Exception
{
ConfigValue value = new ConfigValue( "name", Optional.empty(), Optional.empty(), Optional.empty(),
"description", true, false,
"description", true, false, false,
Optional.empty() );

assertTrue( value.internal() );
Expand All @@ -58,7 +58,7 @@ public void handlesInternal() throws Exception
public void handlesNonEmptyValue() throws Exception
{
ConfigValue value = new ConfigValue( "name", Optional.empty(), Optional.empty(), Optional.of( 1 ),
"description", false, false, Optional.empty() );
"description", false, false, false, Optional.empty() );

assertEquals( Optional.of( 1 ), value.value() );
assertEquals( "1", value.toString() );
Expand All @@ -71,7 +71,7 @@ public void handlesNonEmptyValue() throws Exception
public void handlesDeprecationAndReplacement() throws Exception
{
ConfigValue value = new ConfigValue( "old_name", Optional.empty(), Optional.empty(), Optional.of( 1 ),
"description", false, true,
"description", false, false, true,
Optional.of( "new_name" ) );

assertEquals( Optional.of( 1 ), value.value() );
Expand All @@ -85,7 +85,7 @@ public void handlesDeprecationAndReplacement() throws Exception
public void handlesValueDescription() throws Exception
{
ConfigValue value = new ConfigValue( "old_name", Optional.empty(), Optional.empty(), Optional.of( 1 ),
"a simple integer", false, true,
"a simple integer", false, false, true,
Optional.of( "new_name" ) );

assertEquals( Optional.of( 1 ), value.value() );
Expand Down
Expand Up @@ -79,7 +79,7 @@ public class Config implements DiagnosticsProvider, Configuration
private final Map<String,String> params = new CopyOnWriteHashMap<>(); // Read heavy workload
private final ConfigurationMigrator migrator;
private final List<ConfigurationValidator> validators = new ArrayList<>();
private final Map<String,String> overriddenDefaults = new CopyOnWriteHashMap<>(); // TODO : has bo be reapplied every update
private final Map<String,String> overriddenDefaults = new CopyOnWriteHashMap<>();

// Messages to this log get replayed into a real logger once logging has been instantiated.
private Log log = new BufferingLog();
Expand Down Expand Up @@ -564,6 +564,59 @@ public Optional<?> getValue( @Nonnull String key )
.orElse( Optional.empty() );
}

/**
*
* TODO: DRAGONS! This code works for the current dynamic variables(3), but is not generic.
* TODO: DRAGONS! No migration or config validation is done.
*
* @param setting The setting to set to the specified value.
* @param value The new value to set, passing {@code null} or empty should reset the value back to default value.
* @throws IllegalArgumentException if the provided setting is unknown or not dynamic.
* @throws InvalidSettingException if the value is not formatted correctly.
*/
public void setConfigValue( String setting, String value ) throws IllegalArgumentException, InvalidSettingException
{
// Make sure the setting is valid and is marked as dynamic
Optional<ConfigValue> option =
configOptions.stream().map( it -> it.asConfigValues( params ) ).flatMap( List::stream )
.filter( it -> it.name().equals( setting ) ).findFirst();

if ( !option.isPresent() )
{
throw new IllegalArgumentException( "Unknown setting: " + setting );
}

ConfigValue configValue = option.get();
if ( !configValue.dynamic() )
{
throw new IllegalArgumentException( "Setting is not dynamic and can not be changed at runtime" );
}

if ( value == null || value.isEmpty() )
{
// Empty means we want to delete the configured value and fallback to the default value
params.remove( setting );
if ( overriddenDefaults.containsKey( setting ) )
{
params.put( setting, overriddenDefaults.get( setting ) );
}
}
else
{
// Change setting, make sure it's valid
Map<String,String> newEntry = stringMap( setting, value );
List<SettingValidator> settingValidators = configOptions.stream()
.map( ConfigOptions::settingGroup )
.collect( Collectors.toList() );
for ( SettingValidator validator : settingValidators )
{
validator.validate( newEntry, ignore -> {} ); // Throws if invalid
}

params.put( setting, value );
}
}

/**
* @return all effective config values
*/
Expand Down Expand Up @@ -616,7 +669,7 @@ public void dump( DiagnosticsPhase phase, Logger logger )
private void migrateAndValidateAndUpdateSettings( Map<String,String> settings, boolean warnOnUnknownSettings )
throws InvalidSettingException
{
Map<String,String> migratedSettings = migrator.apply( settings, log );
Map<String,String> migratedSettings = migrateSettings( settings );
params.putAll( migratedSettings );

List<SettingValidator> settingValidators = configOptions.stream()
Expand All @@ -635,6 +688,11 @@ private void migrateAndValidateAndUpdateSettings( Map<String,String> settings, b
}
}

private Map<String,String> migrateSettings( Map<String,String> settings )
{
return migrator.apply( settings, log );
}

private void warnAboutDeprecations( Map<String,String> userSettings )
{
configOptions.stream()
Expand Down
Expand Up @@ -45,6 +45,7 @@
import org.neo4j.kernel.api.proc.UserFunctionSignature;
import org.neo4j.kernel.api.query.ExecutingQuery;
import org.neo4j.kernel.api.security.SecurityContext;
import org.neo4j.kernel.configuration.Config;
import org.neo4j.kernel.impl.api.KernelTransactions;
import org.neo4j.kernel.impl.core.ThreadToStatementContextBridge;
import org.neo4j.kernel.impl.proc.Procedures;
Expand Down Expand Up @@ -265,10 +266,23 @@ public ProcedureResult( ProcedureSignature signature )
private boolean isAdminProcedure( String procedureName )
{
return name.startsWith( "dbms.security." ) && ADMIN_PROCEDURES.contains( procedureName ) ||
name.equals( "dbms.listConfig" );
name.equals( "dbms.listConfig" ) ||
name.equals( "dbms.setConfigValue" );
}
}

@Description( "Updates a given setting value. Passing an empty value will result in removing the configured value " +
"and falling back to the default value. Changes will not persist and will be lost if the server is restarted." )
@Procedure( name = "dbms.setConfigValue", mode = DBMS )
public void setConfigValue( @Name( "setting" ) String setting, @Name( "value" ) String value )
{
securityContext.assertCredentialsNotExpired();
assertAdmin();

Config config = resolver.resolveDependency( Config.class );
config.setConfigValue( setting, value ); // throws if something goes wrong
}

/*
==================================================================================
*/
Expand Down
@@ -0,0 +1,121 @@
/*
* Copyright (c) 2002-2017 "Neo Technology,"
* Network Engine for Objects in Lund AB [http://neotechnology.com]
*
* This file is part of Neo4j.
*
* Neo4j is free software: you can redistribute it and/or modify
* it under the terms of the GNU Affero General Public License as
* published by the Free Software Foundation, either version 3 of the
* License, or (at your option) any later version.
*
* This program is distributed in the hope that it will be useful,
* but WITHOUT ANY WARRANTY; without even the implied warranty of
* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
* GNU Affero General Public License for more details.
*
* You should have received a copy of the GNU Affero General Public License
* along with this program. If not, see <http://www.gnu.org/licenses/>.
*/
package org.neo4j.kernel.enterprise.builtinprocs;

import org.junit.Rule;
import org.junit.Test;

import org.neo4j.graphdb.config.InvalidSettingException;
import org.neo4j.graphdb.factory.GraphDatabaseBuilder;
import org.neo4j.helpers.Exceptions;
import org.neo4j.kernel.configuration.Config;
import org.neo4j.test.rule.DatabaseRule;
import org.neo4j.test.rule.ImpermanentEnterpriseDatabaseRule;
import org.neo4j.test.rule.concurrent.ThreadingRule;

import static org.hamcrest.CoreMatchers.equalTo;
import static org.hamcrest.CoreMatchers.instanceOf;
import static org.junit.Assert.assertFalse;
import static org.junit.Assert.assertThat;
import static org.junit.Assert.assertTrue;
import static org.junit.Assert.fail;
import static org.neo4j.graphdb.factory.GraphDatabaseSettings.cypher_hints_error;
import static org.neo4j.graphdb.factory.GraphDatabaseSettings.log_queries;
import static org.neo4j.graphdb.factory.GraphDatabaseSettings.plugin_dir;

public class SetConfigValueProcedureTest
{
@Rule
public final DatabaseRule db = new ImpermanentEnterpriseDatabaseRule()
{
@Override
protected void configure( GraphDatabaseBuilder builder )
{
builder.setConfig( cypher_hints_error, "true" );
}
};
@Rule
public final ThreadingRule threads = new ThreadingRule();

@Test
public void configShouldBeAffected() throws Exception
{
Config config = db.resolveDependency( Config.class );

db.execute( "CALL dbms.setConfigValue('" + log_queries.name() + "', 'false')" );
assertFalse( config.get( log_queries ) );

db.execute( "CALL dbms.setConfigValue('" + log_queries.name() + "', 'true')" );
assertTrue( config.get( log_queries ) );
}

@Test
public void failIfUnknownSetting() throws Exception
{
try
{
db.execute( "CALL dbms.setConfigValue('unknown.setting.indeed', 'foo')" );
}
catch ( Exception e )
{
Throwable cause = Exceptions.rootCause( e );
assertThat( cause, instanceOf( IllegalArgumentException.class ) );
assertThat( cause.getMessage(), equalTo( "Unknown setting: unknown.setting.indeed" ) );
return;
}
fail( "Should throw IllegalArgumentException" );
}

@Test
public void failIfStaticSetting() throws Exception
{
try
{
// Static setting, at least for now
db.execute( "CALL dbms.setConfigValue('" + plugin_dir.name() + "', 'path/to/dir')" );
}
catch ( Exception e )
{
Throwable cause = Exceptions.rootCause( e );
assertThat( cause, instanceOf( IllegalArgumentException.class ) );
assertThat( cause.getMessage(), equalTo( "Setting is not dynamic and can not be changed at runtime" ) );
return;
}
fail( "Should throw IllegalArgumentException" );
}

@Test
public void failIfInvalidValue() throws Exception
{
try
{
db.execute( "CALL dbms.setConfigValue('" + log_queries.name() + "', 'invalid')" );
}
catch ( Exception e )
{
Throwable cause = Exceptions.rootCause( e );
assertThat( cause, instanceOf( InvalidSettingException.class ) );
assertThat( cause.getMessage(),
equalTo( "Bad value 'invalid' for setting 'dbms.logs.query.enabled': must be 'true' or 'false'" ) );
return;
}
fail( "Should throw InvalidSettingException" );
}
}
Expand Up @@ -361,6 +361,33 @@ public void shouldNotLogPassword() throws Exception
assertThat( logLines.get( 0 ), containsString( neo.subject().username() ) );
}

@Test
public void canBeEnabledAndDisabledAtRuntime() throws Exception
{
GraphDatabaseService database = databaseBuilder.setConfig( GraphDatabaseSettings.log_queries, Settings.FALSE )
.setConfig( GraphDatabaseSettings.log_queries_filename, logFilename.getPath() )
.newGraphDatabase();

database.execute( QUERY ).close();

List<String> strings = readAllLines( logFilename );
assertEquals( 0, strings.size() );

database.execute( "CALL dbms.setConfigValue('" + GraphDatabaseSettings.log_queries.name() + "', 'true')" ).close();
database.execute( QUERY ).close();

// Both config change and query should exist
strings = readAllLines( logFilename );
assertEquals( 2, strings.size() );

database.execute( "CALL dbms.setConfigValue('" + GraphDatabaseSettings.log_queries.name() + "', 'false')" ).close();
database.execute( QUERY ).close();

// Value should not change when disabled
strings = readAllLines( logFilename );
assertEquals( 2, strings.size() );
}

private void executeQueryAndShutdown( GraphDatabaseService database )
{
executeQueryAndShutdown( database, QUERY, Collections.emptyMap() );
Expand Down

0 comments on commit a18079f

Please sign in to comment.