Re: Review Request 17303: Updated getJobs API to return task stats and latest task config
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/17303/#review32720 --- src/main/thrift/org/apache/aurora/gen/api.thrift https://reviews.apache.org/r/17303/#comment61666 I think pre-computing these stats restricts the freedom of an API consumer from displaying them differently. Also, ambiguous names like 'recently' should absolutely be avoided. Before we go down this road it would be good to collect real data showing that this aggregation is necessary on the server. I don't doubt it is, but the exercise will give us a baseline to work with. If it does prove necessary, it would be good to know what contributes to the problem (data transmission, memory, computation time, etc). My intuition is to err on the side of returning more information unless it causes problems; allowing the client to make up its own aggregations. src/main/thrift/org/apache/aurora/gen/api.thrift https://reviews.apache.org/r/17303/#comment61664 This is overloading the purpose of JobConfiguration. It's currently the configuration of a job, and i would prefer to not add use cases to that. It would be much tidier to compose this information _with_ JobConfiguration objects in another struct. - Bill Farner On Jan. 24, 2014, 7:13 a.m., Suman Karumuri wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/17303/ --- (Updated Jan. 24, 2014, 7:13 a.m.) Review request for Aurora, Kevin Sweeney and Bill Farner. Bugs: AURORA-64 https://issues.apache.org/jira/browse/AURORA-64 Repository: aurora Description --- Added task stats to getJobs API so it can be used by the role page in the UI. Refactored code from SchedulerzRole and SchedulerzRoleTest into relevant classes so it can be used by the UI and the thrift API. Added tests for new code. Moved populateJobConfig call into ReadOnlyScheduler. Diffs - src/main/java/org/apache/aurora/scheduler/base/Jobs.java PRE-CREATION src/main/java/org/apache/aurora/scheduler/base/Tasks.java 06a19d80483b6949c9851b5d38fe34ac712aa75e src/main/java/org/apache/aurora/scheduler/http/SchedulerzRole.java b0caca73b46fba928fb718ab45a608dad4685a2f src/main/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterface.java cf9099f307efa23ca34634e3512d9cdbebfa82f2 src/main/thrift/org/apache/aurora/gen/api.thrift 74010379baa2e47cefc228943f766c7b3a8b0d97 src/test/java/org/apache/aurora/scheduler/base/JobsTest.java PRE-CREATION src/test/java/org/apache/aurora/scheduler/base/TaskUtil.java PRE-CREATION src/test/java/org/apache/aurora/scheduler/http/SchedulerzRoleTest.java 912be189583419e7201e45650d18cd24a6a5a35b src/test/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterfaceTest.java 6cefdfad469a9b69a5291ad46be1df14b443472e src/test/resources/org/apache/aurora/gen/api.thrift.md5 42fdca2759f15d007bee058485c237268c57597a Diff: https://reviews.apache.org/r/17303/diff/ Testing --- gradle clean build gradle run to test local UI. Thanks, Suman Karumuri
Review Request 17332: Add a noun supporting operations on roles
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/17332/ --- Review request for Aurora, Kevin Sweeney and Brian Wickman. Bugs: aurora-107 https://issues.apache.org/jira/browse/aurora-107 Repository: aurora Description --- Add a role noun. Currently, the only operation on roles is getting the quota associated with a role, but there are definitely others that can be added later: what packages has a role created? How many jobs is a role running? What privileges are associated with a role? How much system storage is being used by a role? Etc. Diffs - src/main/python/apache/aurora/client/cli/BUILD 8828c1e68b3ae7793fb0bb081730e0ff8fff5ed1 src/main/python/apache/aurora/client/cli/__init__.py 20ecbcf5d3a868f91922244162b516a66d24d32b src/main/python/apache/aurora/client/cli/options.py 1b7155409505b46451df072edd196dd7e4c88f1c src/main/python/apache/aurora/client/cli/role.py PRE-CREATION src/test/python/apache/aurora/client/cli/BUILD f9ebe0cf626a040aa67654faea07b8902e558282 src/test/python/apache/aurora/client/cli/test_get_quota.py PRE-CREATION src/test/python/apache/aurora/client/cli/util.py 76f954377c154d2a7ab3292a2d772b0566ea06fa Diff: https://reviews.apache.org/r/17332/diff/ Testing --- [sun-wukong incubator-aurora (user)]$ ./pants src/test/python/apache/aurora/client/cli:all Build operating on targets: OrderedSet([PythonTestSuite(src/test/python/apache/aurora/client/cli/BUILD:all)]) test session starts platform darwin -- Python 2.7.2 -- pytest-2.5.1 collected 20 items src/test/python/apache/aurora/client/cli/test_create.py src/test/python/apache/aurora/client/cli/test_status.py . src/test/python/apache/aurora/client/cli/test_diff.py ... src/test/python/apache/aurora/client/cli/test_kill.py . src/test/python/apache/aurora/client/cli/test_get_quota.py ... = 20 passed in 0.71 seconds = src.test.python.apache.aurora.client.cli.job . SUCCESS Thanks, Mark Chu-Carroll
Re: Review Request 17332: Add a noun supporting operations on roles
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/17332/#review32757 --- Why is this happening on a role noun and not a quota noun. i.e. aurora quota get west/ksweeney v.s. aurora role get_quota west/ksweeney The second looks inconsistent with the noun-verb model to me and if we ever decide that a quota could be set at a lower level the first is more extensible aurora quota get west/ksweeney/prod aurora quota get west/ksweeney/prod/appserver - Kevin Sweeney On Jan. 24, 2014, 2:07 p.m., Mark Chu-Carroll wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/17332/ --- (Updated Jan. 24, 2014, 2:07 p.m.) Review request for Aurora, Kevin Sweeney and Brian Wickman. Bugs: aurora-107 https://issues.apache.org/jira/browse/aurora-107 Repository: aurora Description --- Add a role noun. Currently, the only operation on roles is getting the quota associated with a role, but there are definitely others that can be added later: what packages has a role created? How many jobs is a role running? What privileges are associated with a role? How much system storage is being used by a role? Etc. Diffs - src/main/python/apache/aurora/client/cli/BUILD 8828c1e68b3ae7793fb0bb081730e0ff8fff5ed1 src/main/python/apache/aurora/client/cli/__init__.py 20ecbcf5d3a868f91922244162b516a66d24d32b src/main/python/apache/aurora/client/cli/options.py 1b7155409505b46451df072edd196dd7e4c88f1c src/main/python/apache/aurora/client/cli/role.py PRE-CREATION src/test/python/apache/aurora/client/cli/BUILD f9ebe0cf626a040aa67654faea07b8902e558282 src/test/python/apache/aurora/client/cli/test_get_quota.py PRE-CREATION src/test/python/apache/aurora/client/cli/util.py 76f954377c154d2a7ab3292a2d772b0566ea06fa Diff: https://reviews.apache.org/r/17332/diff/ Testing --- [sun-wukong incubator-aurora (user)]$ ./pants src/test/python/apache/aurora/client/cli:all Build operating on targets: OrderedSet([PythonTestSuite(src/test/python/apache/aurora/client/cli/BUILD:all)]) test session starts platform darwin -- Python 2.7.2 -- pytest-2.5.1 collected 20 items src/test/python/apache/aurora/client/cli/test_create.py src/test/python/apache/aurora/client/cli/test_status.py . src/test/python/apache/aurora/client/cli/test_diff.py ... src/test/python/apache/aurora/client/cli/test_kill.py . src/test/python/apache/aurora/client/cli/test_get_quota.py ... = 20 passed in 0.71 seconds = src.test.python.apache.aurora.client.cli.job . SUCCESS Thanks, Mark Chu-Carroll
Re: Review Request 17303: Updated getJobs API to return task stats and latest task config
On Jan. 24, 2014, 4:44 p.m., Bill Farner wrote: src/main/thrift/org/apache/aurora/gen/api.thrift, line 187 https://reviews.apache.org/r/17303/diff/1/?file=447779#file447779line187 I think pre-computing these stats restricts the freedom of an API consumer from displaying them differently. Also, ambiguous names like 'recently' should absolutely be avoided. Before we go down this road it would be good to collect real data showing that this aggregation is necessary on the server. I don't doubt it is, but the exercise will give us a baseline to work with. If it does prove necessary, it would be good to know what contributes to the problem (data transmission, memory, computation time, etc). My intuition is to err on the side of returning more information unless it causes problems; allowing the client to make up its own aggregations. Sent out detailed stats in an email. If we send all the task configs(6000) instead of summarizing them, the response size for our largest role would be roughly 900kb to 1.2MB through a conservative estimate. Summarizing all these stats on the server, the response size for the current data is 50-100 bytes. On the server side that is not a lot of compute power, but with several concurrent requests, it may saturate the network. On the client side, that is a lot of data and given that JS is single threaded, it will certainly cause UI hiccups and browser crashes. In this particular case, I feel sending summary data is better, since the users have the ability to query the tasks per job using another API and since most of the data we send is discarded in the current use case. I agree that recently is ambiguous. We can consider renaming it to tasksFailedInLast6Hours or dropping it entirely. I am in favor of the later since: a) We define the status of a service rather arbitrarily. For example, if I have a failed task, that I fixed in a subsequent deployment, the jobs status is still warning for the next 6 hours which is not accurate. b) I am unsure how useful Stable/UnStable and Warning badges are in practice since most services rely on a stats gathering framework to check the health of their service. On Jan. 24, 2014, 4:44 p.m., Bill Farner wrote: src/main/thrift/org/apache/aurora/gen/api.thrift, line 214 https://reviews.apache.org/r/17303/diff/1/?file=447779#file447779line214 This is overloading the purpose of JobConfiguration. It's currently the configuration of a job, and i would prefer to not add use cases to that. It would be much tidier to compose this information _with_ JobConfiguration objects in another struct. Agreed. - Suman --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/17303/#review32720 --- On Jan. 24, 2014, 7:13 a.m., Suman Karumuri wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/17303/ --- (Updated Jan. 24, 2014, 7:13 a.m.) Review request for Aurora, Kevin Sweeney and Bill Farner. Bugs: AURORA-64 https://issues.apache.org/jira/browse/AURORA-64 Repository: aurora Description --- Added task stats to getJobs API so it can be used by the role page in the UI. Refactored code from SchedulerzRole and SchedulerzRoleTest into relevant classes so it can be used by the UI and the thrift API. Added tests for new code. Moved populateJobConfig call into ReadOnlyScheduler. Diffs - src/main/java/org/apache/aurora/scheduler/base/Jobs.java PRE-CREATION src/main/java/org/apache/aurora/scheduler/base/Tasks.java 06a19d80483b6949c9851b5d38fe34ac712aa75e src/main/java/org/apache/aurora/scheduler/http/SchedulerzRole.java b0caca73b46fba928fb718ab45a608dad4685a2f src/main/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterface.java cf9099f307efa23ca34634e3512d9cdbebfa82f2 src/main/thrift/org/apache/aurora/gen/api.thrift 74010379baa2e47cefc228943f766c7b3a8b0d97 src/test/java/org/apache/aurora/scheduler/base/JobsTest.java PRE-CREATION src/test/java/org/apache/aurora/scheduler/base/TaskUtil.java PRE-CREATION src/test/java/org/apache/aurora/scheduler/http/SchedulerzRoleTest.java 912be189583419e7201e45650d18cd24a6a5a35b src/test/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterfaceTest.java 6cefdfad469a9b69a5291ad46be1df14b443472e src/test/resources/org/apache/aurora/gen/api.thrift.md5 42fdca2759f15d007bee058485c237268c57597a Diff: https://reviews.apache.org/r/17303/diff/ Testing --- gradle clean build gradle run to test local UI. Thanks, Suman Karumuri
