Skip to content
This repository has been archived by the owner on Dec 15, 2018. It is now read-only.

Commit

Permalink
changed the default number of shards for bazel projects
Browse files Browse the repository at this point in the history
Summary: This changes the default from 10 to 1, which makes more sense because we are sharding at a target level, and we don't always have that many targets. This number is configurable per project via the config `bazel.num-executors`.

Test Plan: unit test

Reviewers: anupc

Reviewed By: anupc

Subscribers: changesbot, kylec

Differential Revision: https://tails.corp.dropbox.com/D225035
  • Loading branch information
Naphat Sanguansin committed Aug 31, 2016
1 parent 9a4b66c commit d0264b7
Show file tree
Hide file tree
Showing 5 changed files with 54 additions and 1 deletion.
3 changes: 3 additions & 0 deletions changes/config.py
Original file line number Diff line number Diff line change
Expand Up @@ -428,6 +428,9 @@ def create_app(_read_config=True, **config):
# Maximum memory allowed per executor (in MB)
app.config['MAX_MEM_MB_PER_EXECUTOR'] = 16384

# Maximum number of bazel executors allowed.
app.config['MAX_EXECUTORS'] = 10

# Absolute path to Bazel root (passed via --output_root to Bazel)
# Storing bazel cache in tmpfs could be a bad idea because:
# - tmpfs means any files stored here will be stored purely in RAM and will eat into container limits
Expand Down
6 changes: 6 additions & 0 deletions changes/models/jobplan.py
Original file line number Diff line number Diff line change
Expand Up @@ -199,6 +199,7 @@ def get_build_step_for_job(cls, job_id):

bazel_exclude_tags = project_config['bazel.exclude-tags']
bazel_cpus = project_config['bazel.cpus']
bazel_max_executors = project_config['bazel.max-executors']
if bazel_cpus < 1 or bazel_cpus > current_app.config['MAX_CPUS_PER_EXECUTOR']:
logging.error('Project config for project %s requests invalid number of CPUs: constraint 1 <= %d <= %d' % (
job.project.slug,
Expand All @@ -216,6 +217,10 @@ def get_build_step_for_job(cls, job_id):
current_app.config['MAX_MEM_MB_PER_EXECUTOR']))
return jobplan, None

if bazel_max_executors < 1 or bazel_max_executors > current_app.config['MAX_EXECUTORS']:
logging.error('Project config for project %s requests invalid number of executors: constraint 1 <= %d <= %d', job.project.slug, bazel_max_executors, current_app.config['MAX_EXECUTORS'])
return jobplan, None

implementation = LXCBuildStep(
cluster=current_app.config['DEFAULT_CLUSTER'],
commands=[
Expand All @@ -228,6 +233,7 @@ def get_build_step_for_job(cls, job_id):
],
cpus=bazel_cpus,
memory=bazel_memory,
max_executors=bazel_max_executors,
)
return jobplan, implementation

Expand Down
1 change: 1 addition & 0 deletions changes/models/project.py
Original file line number Diff line number Diff line change
Expand Up @@ -65,6 +65,7 @@ def get(cls, id):
'bazel.exclude-tags': ['manual'], # Ignore tests with manual tag
'bazel.cpus': DEFAULT_CPUS,
'bazel.mem': DEFAULT_MEMORY_MB,
'bazel.max-executors': 1,
}

def get_config_path(self):
Expand Down
3 changes: 2 additions & 1 deletion conftest.py
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,8 @@ def app(request, session_config):
DEFAULT_FILE_STORAGE='changes.storage.mock.FileStorageCache',
LXC_PRE_LAUNCH='echo pre',
LXC_POST_LAUNCH='echo post',
SNAPSHOT_S3_BUCKET='snapshot-bucket'
SNAPSHOT_S3_BUCKET='snapshot-bucket',
MAX_EXECUTORS=10,
)
app_context = app.test_request_context()
context = app_context.push()
Expand Down
42 changes: 42 additions & 0 deletions tests/changes/models/test_job.py
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,7 @@ def test_autogenerated_commands(self, get_config):
'bazel.exclude-tags': [],
'bazel.cpus': 4, # Default
'bazel.mem': 8192, # Default
'bazel.max-executors': 1, # Default
}

with mock.patch('changes.utils.bazel_setup.COLLECT_BAZEL_TARGETS') as mock_collect_bazel_target_command:
Expand Down Expand Up @@ -72,6 +73,8 @@ def test_autogenerated_commands(self, get_config):

assert len(implementation.commands) == 3

assert implementation.max_executors == 1

assert implementation.commands[0].type == CommandType.setup
assert implementation.commands[0].script == bazel_setup_expected

Expand Down Expand Up @@ -109,12 +112,15 @@ def test_autogenerated_commands_with_exclusions(self, get_config):
],
'bazel.cpus': 2,
'bazel.mem': 1234,
'bazel.max-executors': 3,
}

with mock.patch('changes.utils.bazel_setup.COLLECT_BAZEL_TARGETS') as mock_collect_bazel_target_command:
mock_collect_bazel_target_command.format.return_value = 'test script'
_, implementation = JobPlan.get_build_step_for_job(self._create_job_and_jobplan().id)

assert implementation.max_executors == 3

assert implementation.resources['cpus'] == 2
assert implementation.resources['mem'] == 1234

Expand Down Expand Up @@ -150,6 +156,7 @@ def test_invalid_cpus(self, get_config):
'bazel.exclude-tags': [],
'bazel.cpus': 0, # 0 CPUs is not valid
'bazel.mem': 8192,
'bazel.max-executors': 1,
}

_, implementation = JobPlan.get_build_step_for_job(self._create_job_and_jobplan().id)
Expand All @@ -163,6 +170,7 @@ def test_invalid_cpus(self, get_config):
'bazel.exclude-tags': [],
'bazel.cpus': 2, # Too many
'bazel.mem': 8192,
'bazel.max-executors': 1,
}

current_app.config['MAX_CPUS_PER_EXECUTOR'] = 1
Expand All @@ -179,6 +187,7 @@ def test_invalid_mems(self, get_config):
'bazel.exclude-tags': [],
'bazel.cpus': 1,
'bazel.mem': 1025, # Too high
'bazel.max-executors': 1,
}

current_app.config['MIN_MEM_MB_PER_EXECUTOR'] = 1
Expand All @@ -195,10 +204,43 @@ def test_invalid_mems(self, get_config):
'bazel.exclude-tags': [],
'bazel.cpus': 1,
'bazel.mem': 1025, # Too low
'bazel.max-executors': 1,
}

current_app.config['MIN_MEM_MB_PER_EXECUTOR'] = 2000
current_app.config['MAX_MEM_MB_PER_EXECUTOR'] = 3000
_, implementation = JobPlan.get_build_step_for_job(self._create_job_and_jobplan().id)

assert implementation is None

@mock.patch('changes.models.project.Project.get_config')
def test_invalid_num_executors(self, get_config):
get_config.return_value = {
'bazel.targets': [
'//aa/bb/cc/...',
'//aa/abc/...',
],
'bazel.exclude-tags': [],
'bazel.cpus': 1,
'bazel.mem': 1234,
'bazel.max-executors': 0, # invalid
}

_, implementation = JobPlan.get_build_step_for_job(self._create_job_and_jobplan().id)

assert implementation is None

get_config.return_value = {
'bazel.targets': [
'//aa/bb/cc/...',
'//aa/abc/...',
],
'bazel.exclude-tags': [],
'bazel.cpus': 1,
'bazel.mem': 1234,
'bazel.max-executors': 11, # too high
}

_, implementation = JobPlan.get_build_step_for_job(self._create_job_and_jobplan().id)

assert implementation is None

0 comments on commit d0264b7

Please sign in to comment.