Skip to content

Commit

Permalink
Make per-user limit on number of jobs dynamic
Browse files Browse the repository at this point in the history
Attempt to fetch the per-value limit from the environment, and fall back to the current or default if an override doesn't exist.
  • Loading branch information
mprimi committed Jan 9, 2019
1 parent 927132c commit aac28e1
Show file tree
Hide file tree
Showing 4 changed files with 70 additions and 6 deletions.
Expand Up @@ -17,13 +17,14 @@
*/
package com.netflix.genie.core.properties;

import com.google.common.collect.Maps;
import lombok.Getter;
import lombok.Setter;
import org.springframework.context.EnvironmentAware;
import org.springframework.core.env.Environment;
import org.springframework.validation.annotation.Validated;

import javax.validation.constraints.Min;
import java.util.Map;
import java.util.concurrent.atomic.AtomicReference;

/**
* Properties related to user limits in number of active jobs.
Expand All @@ -34,7 +35,18 @@
@Getter
@Setter
@Validated
public class JobsUsersActiveLimitProperties {
public class JobsUsersActiveLimitProperties implements EnvironmentAware {

/**
* The property prefix for job user limiting.
*/
public static final String PROPERTY_PREFIX = "genie.jobs.users.activeLimit";

/**
* The property key prefix for per-user limit.
*/
public static final String USER_LIMIT_OVERRIDE_PROPERTY_PREFIX = PROPERTY_PREFIX + ".overrides.";

/**
* Default value for active user job limit enabled.
*/
Expand All @@ -48,7 +60,7 @@ public class JobsUsersActiveLimitProperties {
private boolean enabled = DEFAULT_ENABLED;
@Min(value = 1)
private int count = DEFAULT_COUNT;
private Map<String, Integer> overrides = Maps.newHashMap();
private AtomicReference<Environment> environment = new AtomicReference<>();

/**
* Get the maximum number of jobs a user is allowed to run concurrently.
Expand All @@ -58,6 +70,22 @@ public class JobsUsersActiveLimitProperties {
* @return the maximum number of jobs
*/
public int getUserLimit(final String user) {
return overrides.getOrDefault(user, count);
final Environment env = this.environment.get();
if (env != null) {
return env.getProperty(
USER_LIMIT_OVERRIDE_PROPERTY_PREFIX + user,
Integer.class,
this.count
);
}
return this.count;
}

/**
* {@inheritDoc}
*/
@Override
public void setEnvironment(final Environment environment) {
this.environment.set(environment);
}
}
Expand Up @@ -22,6 +22,8 @@
import org.junit.Before;
import org.junit.Test;
import org.junit.experimental.categories.Category;
import org.mockito.Mockito;
import org.springframework.core.env.Environment;

/**
* Unit tests for JobsUsersActiveLimitProperties.
Expand All @@ -48,6 +50,7 @@ public void setup() {
public void canConstruct() {
Assert.assertEquals(JobsUsersActiveLimitProperties.DEFAULT_ENABLED, this.properties.isEnabled());
Assert.assertEquals(JobsUsersActiveLimitProperties.DEFAULT_COUNT, this.properties.getCount());
Assert.assertEquals(JobsUsersActiveLimitProperties.DEFAULT_COUNT, this.properties.getUserLimit("SomeUser"));
}

/**
Expand All @@ -69,4 +72,24 @@ public void canSetRunAsEnabled() {
this.properties.setCount(newCountValue);
Assert.assertEquals(newCountValue, this.properties.getCount());
}

/**
* Make sure environment is used when looking for a user-specific limit override.
*/
@Test
public void testUserOverrides() {
final String userName = "SomeUser";
final int userLimit = 999;
final Environment environment = Mockito.mock(Environment.class);
Mockito
.when(environment.getProperty(
JobsUsersActiveLimitProperties.USER_LIMIT_OVERRIDE_PROPERTY_PREFIX + userName,
Integer.class,
this.properties.getCount())
)
.thenReturn(userLimit);
this.properties.setEnvironment(environment);

Assert.assertEquals(userLimit, this.properties.getUserLimit(userName));
}
}
2 changes: 1 addition & 1 deletion genie-docs/src/docs/asciidoc/_properties.adoc
Expand Up @@ -139,7 +139,7 @@ to work.
|100

|genie.jobs.users.activeLimit.overrides.<user-name>
|The maximum number of active jobs that user 'user-name' is allowed to have. This property is ignored unless `genie.jobs.users.activeLimit.enabled` is set to true.
|(Dynamic) The maximum number of active jobs that user 'user-name' is allowed to have. This property is ignored unless `genie.jobs.users.activeLimit.enabled` is set to true.
|-

|genie.jobs.completionCheckBackOff.minInterval
Expand Down
Expand Up @@ -23,11 +23,13 @@
import com.netflix.genie.test.categories.IntegrationTest;
import org.hamcrest.Matchers;
import org.junit.Assert;
import org.junit.Before;
import org.junit.Test;
import org.junit.experimental.categories.Category;
import org.junit.runner.RunWith;
import org.springframework.beans.factory.annotation.Autowired;
import org.springframework.boot.test.context.SpringBootTest;
import org.springframework.core.env.Environment;
import org.springframework.test.context.TestPropertySource;
import org.springframework.test.context.junit4.SpringRunner;

Expand All @@ -52,6 +54,17 @@ public class PropertiesConfigIntegrationTest {
@Autowired
private HealthProperties healthProperties;

@Autowired
private Environment environment;

/**
* Set up.
*/
@Before
public void setup() {
jobsProperties.getUsers().getActiveLimit().setEnvironment(environment);
}

/**
* Verify than beans get autowired, and that (non-default) values correspond to the expected set via properties
* file.
Expand Down

0 comments on commit aac28e1

Please sign in to comment.