Re: Review Request 20950: Moving kill wait to the client (Part 1: client changes)
On May 1, 2014, 8:59 p.m., Brian Wickman wrote: src/main/python/apache/aurora/client/api/job_monitor.py, line 81 https://reviews.apache.org/r/20950/diff/2/?file=572973#file572973line81 = Maxim Khutornenko wrote: It's safe as is but since Mark asked about this too changing it for better readability. Brian Wickman wrote: it's not safe. you assume that min interval and max interval are powers of 2 of each other. plus then you run into python LOLisms like this mba=aurora=; python Python 2.6.9 (unknown, Apr 15 2014, 11:11:58) [GCC 4.2.1 (Based on Apple Inc. build 5658) (LLVM build 2336.11.00)] on darwin Type help, copyright, credits or license for more information. 1.1 1.1001 3 * 1.1 == 3.3 False Maxim Khutornenko wrote: Completely agree in case of floats. However, I'd still say it was safe considering that Amount does not accept floats and especially this line: poll_interval = min(self._max_poll_interval, 2 * poll_interval) Anyway, I changed it to = to avoid further questions :) derp - Brian --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/20950/#review41947 --- On May 1, 2014, 10:21 p.m., Maxim Khutornenko wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/20950/ --- (Updated May 1, 2014, 10:21 p.m.) Review request for Aurora, Mark Chu-Carroll and Brian Wickman. Bugs: AURORA-370 https://issues.apache.org/jira/browse/AURORA-370 Repository: aurora Description --- Migration plan: 1. Prepare client to poll for the status of tasks being killed. This is mostly a noop at this point while the scheduler wait with a shorter timeout is still in place. 2. Cut off scheduler killTasks wait thus relying on the client poll (coming next). This RB addresses client changes: - Repurposing JobMonitor to poll for all job tasks regardless of their arrival time. - Introducing a timeout cap into JobMonitor to interrupt wait upon reaching the max_poll_interval. - Setting the JobMonitor max_poll_interval to be higher than the scheduler killTasks backoff to ensure smooth migration. - Added wait_util calls into kill/killall/updater calls. Once this ticket is closed, the next step would be to make kill/killall truly async: AURORA-371 Diffs - src/main/python/apache/aurora/client/api/BUILD 32097d220a6f8a58555260ce081ace0c58b9fa99 src/main/python/apache/aurora/client/api/job_monitor.py b694ef62ba708c4b67c5cc931b0edfd93702027f src/main/python/apache/aurora/client/api/updater.py 0acf45034db124ed369d0bcb8095fe5545605eed src/main/python/apache/aurora/client/cli/jobs.py 782b34800def55c54b67ca5f13da66c4997a0777 src/main/python/apache/aurora/client/cli/task.py 62747ed3e3f5f6ecd9b9c46f6f175c634c068358 src/main/python/apache/aurora/client/commands/core.py 39190e06f3eac1ab4dabf48055418bc383be14ed src/test/python/apache/aurora/client/api/BUILD dd9b79764db0bbc3a11c51051e43b93a9b5d370a src/test/python/apache/aurora/client/api/test_job_monitor.py 32609e4e97afc7b8b164f3db438c60efd0ebbe11 src/test/python/apache/aurora/client/api/test_updater.py e7eb1e783baaabe9437e55ecb848f6a42de13dd7 src/test/python/apache/aurora/client/cli/test_command_hooks.py 7c6f70c7ef7534e9dd4986364331c67f6f7c36b1 src/test/python/apache/aurora/client/cli/test_create.py 875573e2ee534ea50da71fac2174ecc8877a714d src/test/python/apache/aurora/client/cli/test_kill.py cf5df648206e0cc234d603c1fd56dc7ab1311fa9 src/test/python/apache/aurora/client/cli/test_plugins.py 2dab749d1dcba99a1e3c792c8f26ee86441673d1 src/test/python/apache/aurora/client/cli/test_update.py cf077e821ebe32104b3e1345014f5c7b07c44c34 src/test/python/apache/aurora/client/cli/util.py 2225ab071dff72be547c43f66ffdc890171c3288 src/test/python/apache/aurora/client/commands/test_create.py e0ecb523db4ce3b1901adcc96a2fd5ce8184f621 src/test/python/apache/aurora/client/commands/test_kill.py db820a543b0ea11e5a40cc2a00209aa8fa6186c9 src/test/python/apache/aurora/client/commands/test_update.py 6e145db9ff8213c8f098b53b9a9668532d664249 Diff: https://reviews.apache.org/r/20950/diff/ Testing --- ./pants src/test/python:all Thanks, Maxim Khutornenko
Re: Review Request 20950: Moving kill wait to the client (Part 1: client changes)
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/20950/#review42025 --- Ship it! Ship It! - Brian Wickman On May 1, 2014, 10:21 p.m., Maxim Khutornenko wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/20950/ --- (Updated May 1, 2014, 10:21 p.m.) Review request for Aurora, Mark Chu-Carroll and Brian Wickman. Bugs: AURORA-370 https://issues.apache.org/jira/browse/AURORA-370 Repository: aurora Description --- Migration plan: 1. Prepare client to poll for the status of tasks being killed. This is mostly a noop at this point while the scheduler wait with a shorter timeout is still in place. 2. Cut off scheduler killTasks wait thus relying on the client poll (coming next). This RB addresses client changes: - Repurposing JobMonitor to poll for all job tasks regardless of their arrival time. - Introducing a timeout cap into JobMonitor to interrupt wait upon reaching the max_poll_interval. - Setting the JobMonitor max_poll_interval to be higher than the scheduler killTasks backoff to ensure smooth migration. - Added wait_util calls into kill/killall/updater calls. Once this ticket is closed, the next step would be to make kill/killall truly async: AURORA-371 Diffs - src/main/python/apache/aurora/client/api/BUILD 32097d220a6f8a58555260ce081ace0c58b9fa99 src/main/python/apache/aurora/client/api/job_monitor.py b694ef62ba708c4b67c5cc931b0edfd93702027f src/main/python/apache/aurora/client/api/updater.py 0acf45034db124ed369d0bcb8095fe5545605eed src/main/python/apache/aurora/client/cli/jobs.py 782b34800def55c54b67ca5f13da66c4997a0777 src/main/python/apache/aurora/client/cli/task.py 62747ed3e3f5f6ecd9b9c46f6f175c634c068358 src/main/python/apache/aurora/client/commands/core.py 39190e06f3eac1ab4dabf48055418bc383be14ed src/test/python/apache/aurora/client/api/BUILD dd9b79764db0bbc3a11c51051e43b93a9b5d370a src/test/python/apache/aurora/client/api/test_job_monitor.py 32609e4e97afc7b8b164f3db438c60efd0ebbe11 src/test/python/apache/aurora/client/api/test_updater.py e7eb1e783baaabe9437e55ecb848f6a42de13dd7 src/test/python/apache/aurora/client/cli/test_command_hooks.py 7c6f70c7ef7534e9dd4986364331c67f6f7c36b1 src/test/python/apache/aurora/client/cli/test_create.py 875573e2ee534ea50da71fac2174ecc8877a714d src/test/python/apache/aurora/client/cli/test_kill.py cf5df648206e0cc234d603c1fd56dc7ab1311fa9 src/test/python/apache/aurora/client/cli/test_plugins.py 2dab749d1dcba99a1e3c792c8f26ee86441673d1 src/test/python/apache/aurora/client/cli/test_update.py cf077e821ebe32104b3e1345014f5c7b07c44c34 src/test/python/apache/aurora/client/cli/util.py 2225ab071dff72be547c43f66ffdc890171c3288 src/test/python/apache/aurora/client/commands/test_create.py e0ecb523db4ce3b1901adcc96a2fd5ce8184f621 src/test/python/apache/aurora/client/commands/test_kill.py db820a543b0ea11e5a40cc2a00209aa8fa6186c9 src/test/python/apache/aurora/client/commands/test_update.py 6e145db9ff8213c8f098b53b9a9668532d664249 Diff: https://reviews.apache.org/r/20950/diff/ Testing --- ./pants src/test/python:all Thanks, Maxim Khutornenko
Re: Review Request 20950: Moving kill wait to the client (Part 1: client changes)
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/20950/#review41938 --- src/main/python/apache/aurora/client/api/job_monitor.py https://reviews.apache.org/r/20950/#comment75594 shouldn't this be =? src/test/python/apache/aurora/client/api/test_job_monitor.py https://reviews.apache.org/r/20950/#comment75595 You can just assert monitor.wait_until - Mark Chu-Carroll On May 1, 2014, 1:01 p.m., Maxim Khutornenko wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/20950/ --- (Updated May 1, 2014, 1:01 p.m.) Review request for Aurora, Mark Chu-Carroll and Brian Wickman. Bugs: AURORA-370 https://issues.apache.org/jira/browse/AURORA-370 Repository: aurora Description --- Migration plan: 1. Prepare client to poll for the status of tasks being killed. This is mostly a noop at this point while the scheduler wait with a shorter timeout is still in place. 2. Cut off scheduler killTasks wait thus relying on the client poll (coming next). This RB addresses client changes: - Repurposing JobMonitor to poll for all job tasks regardless of their arrival time. - Introducing a timeout cap into JobMonitor to interrupt wait upon reaching the max_poll_interval. - Setting the JobMonitor max_poll_interval to be higher than the scheduler killTasks backoff to ensure smooth migration. - Added wait_util calls into kill/killall/updater calls. Once this ticket is closed, the next step would be to make kill/killall truly async: AURORA-371 Diffs - src/main/python/apache/aurora/client/api/BUILD 32097d220a6f8a58555260ce081ace0c58b9fa99 src/main/python/apache/aurora/client/api/job_monitor.py b694ef62ba708c4b67c5cc931b0edfd93702027f src/main/python/apache/aurora/client/api/updater.py 0acf45034db124ed369d0bcb8095fe5545605eed src/main/python/apache/aurora/client/cli/jobs.py 782b34800def55c54b67ca5f13da66c4997a0777 src/main/python/apache/aurora/client/cli/task.py 62747ed3e3f5f6ecd9b9c46f6f175c634c068358 src/main/python/apache/aurora/client/commands/core.py 39190e06f3eac1ab4dabf48055418bc383be14ed src/test/python/apache/aurora/client/api/BUILD dd9b79764db0bbc3a11c51051e43b93a9b5d370a src/test/python/apache/aurora/client/api/test_job_monitor.py 32609e4e97afc7b8b164f3db438c60efd0ebbe11 src/test/python/apache/aurora/client/api/test_updater.py e7eb1e783baaabe9437e55ecb848f6a42de13dd7 src/test/python/apache/aurora/client/cli/test_command_hooks.py 7c6f70c7ef7534e9dd4986364331c67f6f7c36b1 src/test/python/apache/aurora/client/cli/test_create.py 875573e2ee534ea50da71fac2174ecc8877a714d src/test/python/apache/aurora/client/cli/test_kill.py cf5df648206e0cc234d603c1fd56dc7ab1311fa9 src/test/python/apache/aurora/client/cli/test_plugins.py 2dab749d1dcba99a1e3c792c8f26ee86441673d1 src/test/python/apache/aurora/client/cli/test_update.py cf077e821ebe32104b3e1345014f5c7b07c44c34 src/test/python/apache/aurora/client/cli/util.py 2225ab071dff72be547c43f66ffdc890171c3288 src/test/python/apache/aurora/client/commands/test_create.py e0ecb523db4ce3b1901adcc96a2fd5ce8184f621 src/test/python/apache/aurora/client/commands/test_kill.py db820a543b0ea11e5a40cc2a00209aa8fa6186c9 src/test/python/apache/aurora/client/commands/test_update.py 6e145db9ff8213c8f098b53b9a9668532d664249 Diff: https://reviews.apache.org/r/20950/diff/ Testing --- ./pants src/test/python:all Thanks, Maxim Khutornenko
Re: Review Request 20950: Moving kill wait to the client (Part 1: client changes)
On May 1, 2014, 7:44 p.m., Mark Chu-Carroll wrote: src/main/python/apache/aurora/client/api/job_monitor.py, line 81 https://reviews.apache.org/r/20950/diff/1/?file=572657#file572657line81 shouldn't this be =? Line 84 guarantees it never exceeds the max value: poll_interval = min(self.MAX_POLL_INTERVAL, 2 * poll_interval) On May 1, 2014, 7:44 p.m., Mark Chu-Carroll wrote: src/test/python/apache/aurora/client/api/test_job_monitor.py, line 103 https://reviews.apache.org/r/20950/diff/1/?file=572663#file572663line103 You can just assert monitor.wait_until Sure, changed. - Maxim --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/20950/#review41938 --- On May 1, 2014, 5:01 p.m., Maxim Khutornenko wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/20950/ --- (Updated May 1, 2014, 5:01 p.m.) Review request for Aurora, Mark Chu-Carroll and Brian Wickman. Bugs: AURORA-370 https://issues.apache.org/jira/browse/AURORA-370 Repository: aurora Description --- Migration plan: 1. Prepare client to poll for the status of tasks being killed. This is mostly a noop at this point while the scheduler wait with a shorter timeout is still in place. 2. Cut off scheduler killTasks wait thus relying on the client poll (coming next). This RB addresses client changes: - Repurposing JobMonitor to poll for all job tasks regardless of their arrival time. - Introducing a timeout cap into JobMonitor to interrupt wait upon reaching the max_poll_interval. - Setting the JobMonitor max_poll_interval to be higher than the scheduler killTasks backoff to ensure smooth migration. - Added wait_util calls into kill/killall/updater calls. Once this ticket is closed, the next step would be to make kill/killall truly async: AURORA-371 Diffs - src/main/python/apache/aurora/client/api/BUILD 32097d220a6f8a58555260ce081ace0c58b9fa99 src/main/python/apache/aurora/client/api/job_monitor.py b694ef62ba708c4b67c5cc931b0edfd93702027f src/main/python/apache/aurora/client/api/updater.py 0acf45034db124ed369d0bcb8095fe5545605eed src/main/python/apache/aurora/client/cli/jobs.py 782b34800def55c54b67ca5f13da66c4997a0777 src/main/python/apache/aurora/client/cli/task.py 62747ed3e3f5f6ecd9b9c46f6f175c634c068358 src/main/python/apache/aurora/client/commands/core.py 39190e06f3eac1ab4dabf48055418bc383be14ed src/test/python/apache/aurora/client/api/BUILD dd9b79764db0bbc3a11c51051e43b93a9b5d370a src/test/python/apache/aurora/client/api/test_job_monitor.py 32609e4e97afc7b8b164f3db438c60efd0ebbe11 src/test/python/apache/aurora/client/api/test_updater.py e7eb1e783baaabe9437e55ecb848f6a42de13dd7 src/test/python/apache/aurora/client/cli/test_command_hooks.py 7c6f70c7ef7534e9dd4986364331c67f6f7c36b1 src/test/python/apache/aurora/client/cli/test_create.py 875573e2ee534ea50da71fac2174ecc8877a714d src/test/python/apache/aurora/client/cli/test_kill.py cf5df648206e0cc234d603c1fd56dc7ab1311fa9 src/test/python/apache/aurora/client/cli/test_plugins.py 2dab749d1dcba99a1e3c792c8f26ee86441673d1 src/test/python/apache/aurora/client/cli/test_update.py cf077e821ebe32104b3e1345014f5c7b07c44c34 src/test/python/apache/aurora/client/cli/util.py 2225ab071dff72be547c43f66ffdc890171c3288 src/test/python/apache/aurora/client/commands/test_create.py e0ecb523db4ce3b1901adcc96a2fd5ce8184f621 src/test/python/apache/aurora/client/commands/test_kill.py db820a543b0ea11e5a40cc2a00209aa8fa6186c9 src/test/python/apache/aurora/client/commands/test_update.py 6e145db9ff8213c8f098b53b9a9668532d664249 Diff: https://reviews.apache.org/r/20950/diff/ Testing --- ./pants src/test/python:all Thanks, Maxim Khutornenko
Re: Review Request 20950: Moving kill wait to the client (Part 1: client changes)
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/20950/#review41943 --- Ship it! Ship It! - Mark Chu-Carroll On May 1, 2014, 4:42 p.m., Maxim Khutornenko wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/20950/ --- (Updated May 1, 2014, 4:42 p.m.) Review request for Aurora, Mark Chu-Carroll and Brian Wickman. Bugs: AURORA-370 https://issues.apache.org/jira/browse/AURORA-370 Repository: aurora Description --- Migration plan: 1. Prepare client to poll for the status of tasks being killed. This is mostly a noop at this point while the scheduler wait with a shorter timeout is still in place. 2. Cut off scheduler killTasks wait thus relying on the client poll (coming next). This RB addresses client changes: - Repurposing JobMonitor to poll for all job tasks regardless of their arrival time. - Introducing a timeout cap into JobMonitor to interrupt wait upon reaching the max_poll_interval. - Setting the JobMonitor max_poll_interval to be higher than the scheduler killTasks backoff to ensure smooth migration. - Added wait_util calls into kill/killall/updater calls. Once this ticket is closed, the next step would be to make kill/killall truly async: AURORA-371 Diffs - src/main/python/apache/aurora/client/api/BUILD 32097d220a6f8a58555260ce081ace0c58b9fa99 src/main/python/apache/aurora/client/api/job_monitor.py b694ef62ba708c4b67c5cc931b0edfd93702027f src/main/python/apache/aurora/client/api/updater.py 0acf45034db124ed369d0bcb8095fe5545605eed src/main/python/apache/aurora/client/cli/jobs.py 782b34800def55c54b67ca5f13da66c4997a0777 src/main/python/apache/aurora/client/cli/task.py 62747ed3e3f5f6ecd9b9c46f6f175c634c068358 src/main/python/apache/aurora/client/commands/core.py 39190e06f3eac1ab4dabf48055418bc383be14ed src/test/python/apache/aurora/client/api/BUILD dd9b79764db0bbc3a11c51051e43b93a9b5d370a src/test/python/apache/aurora/client/api/test_job_monitor.py 32609e4e97afc7b8b164f3db438c60efd0ebbe11 src/test/python/apache/aurora/client/api/test_updater.py e7eb1e783baaabe9437e55ecb848f6a42de13dd7 src/test/python/apache/aurora/client/cli/test_command_hooks.py 7c6f70c7ef7534e9dd4986364331c67f6f7c36b1 src/test/python/apache/aurora/client/cli/test_create.py 875573e2ee534ea50da71fac2174ecc8877a714d src/test/python/apache/aurora/client/cli/test_kill.py cf5df648206e0cc234d603c1fd56dc7ab1311fa9 src/test/python/apache/aurora/client/cli/test_plugins.py 2dab749d1dcba99a1e3c792c8f26ee86441673d1 src/test/python/apache/aurora/client/cli/test_update.py cf077e821ebe32104b3e1345014f5c7b07c44c34 src/test/python/apache/aurora/client/cli/util.py 2225ab071dff72be547c43f66ffdc890171c3288 src/test/python/apache/aurora/client/commands/test_create.py e0ecb523db4ce3b1901adcc96a2fd5ce8184f621 src/test/python/apache/aurora/client/commands/test_kill.py db820a543b0ea11e5a40cc2a00209aa8fa6186c9 src/test/python/apache/aurora/client/commands/test_update.py 6e145db9ff8213c8f098b53b9a9668532d664249 Diff: https://reviews.apache.org/r/20950/diff/ Testing --- ./pants src/test/python:all Thanks, Maxim Khutornenko
Re: Review Request 20950: Moving kill wait to the client (Part 1: client changes)
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/20950/ --- (Updated May 1, 2014, 8:42 p.m.) Review request for Aurora, Mark Chu-Carroll and Brian Wickman. Changes --- CR comments. Bugs: AURORA-370 https://issues.apache.org/jira/browse/AURORA-370 Repository: aurora Description --- Migration plan: 1. Prepare client to poll for the status of tasks being killed. This is mostly a noop at this point while the scheduler wait with a shorter timeout is still in place. 2. Cut off scheduler killTasks wait thus relying on the client poll (coming next). This RB addresses client changes: - Repurposing JobMonitor to poll for all job tasks regardless of their arrival time. - Introducing a timeout cap into JobMonitor to interrupt wait upon reaching the max_poll_interval. - Setting the JobMonitor max_poll_interval to be higher than the scheduler killTasks backoff to ensure smooth migration. - Added wait_util calls into kill/killall/updater calls. Once this ticket is closed, the next step would be to make kill/killall truly async: AURORA-371 Diffs (updated) - src/main/python/apache/aurora/client/api/BUILD 32097d220a6f8a58555260ce081ace0c58b9fa99 src/main/python/apache/aurora/client/api/job_monitor.py b694ef62ba708c4b67c5cc931b0edfd93702027f src/main/python/apache/aurora/client/api/updater.py 0acf45034db124ed369d0bcb8095fe5545605eed src/main/python/apache/aurora/client/cli/jobs.py 782b34800def55c54b67ca5f13da66c4997a0777 src/main/python/apache/aurora/client/cli/task.py 62747ed3e3f5f6ecd9b9c46f6f175c634c068358 src/main/python/apache/aurora/client/commands/core.py 39190e06f3eac1ab4dabf48055418bc383be14ed src/test/python/apache/aurora/client/api/BUILD dd9b79764db0bbc3a11c51051e43b93a9b5d370a src/test/python/apache/aurora/client/api/test_job_monitor.py 32609e4e97afc7b8b164f3db438c60efd0ebbe11 src/test/python/apache/aurora/client/api/test_updater.py e7eb1e783baaabe9437e55ecb848f6a42de13dd7 src/test/python/apache/aurora/client/cli/test_command_hooks.py 7c6f70c7ef7534e9dd4986364331c67f6f7c36b1 src/test/python/apache/aurora/client/cli/test_create.py 875573e2ee534ea50da71fac2174ecc8877a714d src/test/python/apache/aurora/client/cli/test_kill.py cf5df648206e0cc234d603c1fd56dc7ab1311fa9 src/test/python/apache/aurora/client/cli/test_plugins.py 2dab749d1dcba99a1e3c792c8f26ee86441673d1 src/test/python/apache/aurora/client/cli/test_update.py cf077e821ebe32104b3e1345014f5c7b07c44c34 src/test/python/apache/aurora/client/cli/util.py 2225ab071dff72be547c43f66ffdc890171c3288 src/test/python/apache/aurora/client/commands/test_create.py e0ecb523db4ce3b1901adcc96a2fd5ce8184f621 src/test/python/apache/aurora/client/commands/test_kill.py db820a543b0ea11e5a40cc2a00209aa8fa6186c9 src/test/python/apache/aurora/client/commands/test_update.py 6e145db9ff8213c8f098b53b9a9668532d664249 Diff: https://reviews.apache.org/r/20950/diff/ Testing --- ./pants src/test/python:all Thanks, Maxim Khutornenko
Re: Review Request 20950: Moving kill wait to the client (Part 1: client changes)
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/20950/#review41947 --- src/main/python/apache/aurora/client/api/job_monitor.py https://reviews.apache.org/r/20950/#comment75607 i'm fine with tuning the min_poll_interval and max_poll_interval but possibly make them configurable via __init__, ie. __init__(..., min_poll_interval=MIN_POLL_INTERVAL, max_poll_interval=MAX_POLL_INTERVAL) src/main/python/apache/aurora/client/api/job_monitor.py https://reviews.apache.org/r/20950/#comment75608 as a library this should probably be log.error src/main/python/apache/aurora/client/api/job_monitor.py https://reviews.apache.org/r/20950/#comment75610 update the docstring to reflect new return behavior src/main/python/apache/aurora/client/api/job_monitor.py https://reviews.apache.org/r/20950/#comment75611 i don't like the idea of wait_until having side effects -- this effectively renders it not thread safe. can you clone the query and update instanceIds on the cloned query, then change iter_query to take a query parameter? src/main/python/apache/aurora/client/api/job_monitor.py https://reviews.apache.org/r/20950/#comment75614 = src/main/python/apache/aurora/client/api/updater.py https://reviews.apache.org/r/20950/#comment75612 pass the kwarg so that this is more self-documenting, i.e. with_timeout=True src/main/python/apache/aurora/client/commands/core.py https://reviews.apache.org/r/20950/#comment75615 weird indent. also this should use die('Tasks were not killed in time.') - Brian Wickman On May 1, 2014, 8:42 p.m., Maxim Khutornenko wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/20950/ --- (Updated May 1, 2014, 8:42 p.m.) Review request for Aurora, Mark Chu-Carroll and Brian Wickman. Bugs: AURORA-370 https://issues.apache.org/jira/browse/AURORA-370 Repository: aurora Description --- Migration plan: 1. Prepare client to poll for the status of tasks being killed. This is mostly a noop at this point while the scheduler wait with a shorter timeout is still in place. 2. Cut off scheduler killTasks wait thus relying on the client poll (coming next). This RB addresses client changes: - Repurposing JobMonitor to poll for all job tasks regardless of their arrival time. - Introducing a timeout cap into JobMonitor to interrupt wait upon reaching the max_poll_interval. - Setting the JobMonitor max_poll_interval to be higher than the scheduler killTasks backoff to ensure smooth migration. - Added wait_util calls into kill/killall/updater calls. Once this ticket is closed, the next step would be to make kill/killall truly async: AURORA-371 Diffs - src/main/python/apache/aurora/client/api/BUILD 32097d220a6f8a58555260ce081ace0c58b9fa99 src/main/python/apache/aurora/client/api/job_monitor.py b694ef62ba708c4b67c5cc931b0edfd93702027f src/main/python/apache/aurora/client/api/updater.py 0acf45034db124ed369d0bcb8095fe5545605eed src/main/python/apache/aurora/client/cli/jobs.py 782b34800def55c54b67ca5f13da66c4997a0777 src/main/python/apache/aurora/client/cli/task.py 62747ed3e3f5f6ecd9b9c46f6f175c634c068358 src/main/python/apache/aurora/client/commands/core.py 39190e06f3eac1ab4dabf48055418bc383be14ed src/test/python/apache/aurora/client/api/BUILD dd9b79764db0bbc3a11c51051e43b93a9b5d370a src/test/python/apache/aurora/client/api/test_job_monitor.py 32609e4e97afc7b8b164f3db438c60efd0ebbe11 src/test/python/apache/aurora/client/api/test_updater.py e7eb1e783baaabe9437e55ecb848f6a42de13dd7 src/test/python/apache/aurora/client/cli/test_command_hooks.py 7c6f70c7ef7534e9dd4986364331c67f6f7c36b1 src/test/python/apache/aurora/client/cli/test_create.py 875573e2ee534ea50da71fac2174ecc8877a714d src/test/python/apache/aurora/client/cli/test_kill.py cf5df648206e0cc234d603c1fd56dc7ab1311fa9 src/test/python/apache/aurora/client/cli/test_plugins.py 2dab749d1dcba99a1e3c792c8f26ee86441673d1 src/test/python/apache/aurora/client/cli/test_update.py cf077e821ebe32104b3e1345014f5c7b07c44c34 src/test/python/apache/aurora/client/cli/util.py 2225ab071dff72be547c43f66ffdc890171c3288 src/test/python/apache/aurora/client/commands/test_create.py e0ecb523db4ce3b1901adcc96a2fd5ce8184f621 src/test/python/apache/aurora/client/commands/test_kill.py db820a543b0ea11e5a40cc2a00209aa8fa6186c9 src/test/python/apache/aurora/client/commands/test_update.py 6e145db9ff8213c8f098b53b9a9668532d664249 Diff: https://reviews.apache.org/r/20950/diff/ Testing --- ./pants src/test/python:all Thanks,
Re: Review Request 20950: Moving kill wait to the client (Part 1: client changes)
On May 1, 2014, 8:59 p.m., Brian Wickman wrote: src/main/python/apache/aurora/client/api/job_monitor.py, line 81 https://reviews.apache.org/r/20950/diff/2/?file=572973#file572973line81 = It's safe as is but since Mark asked about this too changing it for better readability. On May 1, 2014, 8:59 p.m., Brian Wickman wrote: src/main/python/apache/aurora/client/api/job_monitor.py, line 44 https://reviews.apache.org/r/20950/diff/2/?file=572973#file572973line44 i'm fine with tuning the min_poll_interval and max_poll_interval but possibly make them configurable via __init__, ie. __init__(..., min_poll_interval=MIN_POLL_INTERVAL, max_poll_interval=MAX_POLL_INTERVAL) Sure, works for me. On May 1, 2014, 8:59 p.m., Brian Wickman wrote: src/main/python/apache/aurora/client/api/job_monitor.py, line 56 https://reviews.apache.org/r/20950/diff/2/?file=572973#file572973line56 as a library this should probably be log.error Done. On May 1, 2014, 8:59 p.m., Brian Wickman wrote: src/main/python/apache/aurora/client/api/job_monitor.py, lines 74-75 https://reviews.apache.org/r/20950/diff/2/?file=572973#file572973line74 update the docstring to reflect new return behavior Done. On May 1, 2014, 8:59 p.m., Brian Wickman wrote: src/main/python/apache/aurora/client/api/job_monitor.py, lines 76-77 https://reviews.apache.org/r/20950/diff/2/?file=572973#file572973line76 i don't like the idea of wait_until having side effects -- this effectively renders it not thread safe. can you clone the query and update instanceIds on the cloned query, then change iter_query to take a query parameter? Great catch. Fixed. On May 1, 2014, 8:59 p.m., Brian Wickman wrote: src/main/python/apache/aurora/client/api/updater.py, line 284 https://reviews.apache.org/r/20950/diff/2/?file=572974#file572974line284 pass the kwarg so that this is more self-documenting, i.e. with_timeout=True Done. On May 1, 2014, 8:59 p.m., Brian Wickman wrote: src/main/python/apache/aurora/client/commands/core.py, lines 101-102 https://reviews.apache.org/r/20950/diff/2/?file=572977#file572977line101 weird indent. also this should use die('Tasks were not killed in time.') Done and done. - Maxim --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/20950/#review41947 --- On May 1, 2014, 8:42 p.m., Maxim Khutornenko wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/20950/ --- (Updated May 1, 2014, 8:42 p.m.) Review request for Aurora, Mark Chu-Carroll and Brian Wickman. Bugs: AURORA-370 https://issues.apache.org/jira/browse/AURORA-370 Repository: aurora Description --- Migration plan: 1. Prepare client to poll for the status of tasks being killed. This is mostly a noop at this point while the scheduler wait with a shorter timeout is still in place. 2. Cut off scheduler killTasks wait thus relying on the client poll (coming next). This RB addresses client changes: - Repurposing JobMonitor to poll for all job tasks regardless of their arrival time. - Introducing a timeout cap into JobMonitor to interrupt wait upon reaching the max_poll_interval. - Setting the JobMonitor max_poll_interval to be higher than the scheduler killTasks backoff to ensure smooth migration. - Added wait_util calls into kill/killall/updater calls. Once this ticket is closed, the next step would be to make kill/killall truly async: AURORA-371 Diffs - src/main/python/apache/aurora/client/api/BUILD 32097d220a6f8a58555260ce081ace0c58b9fa99 src/main/python/apache/aurora/client/api/job_monitor.py b694ef62ba708c4b67c5cc931b0edfd93702027f src/main/python/apache/aurora/client/api/updater.py 0acf45034db124ed369d0bcb8095fe5545605eed src/main/python/apache/aurora/client/cli/jobs.py 782b34800def55c54b67ca5f13da66c4997a0777 src/main/python/apache/aurora/client/cli/task.py 62747ed3e3f5f6ecd9b9c46f6f175c634c068358 src/main/python/apache/aurora/client/commands/core.py 39190e06f3eac1ab4dabf48055418bc383be14ed src/test/python/apache/aurora/client/api/BUILD dd9b79764db0bbc3a11c51051e43b93a9b5d370a src/test/python/apache/aurora/client/api/test_job_monitor.py 32609e4e97afc7b8b164f3db438c60efd0ebbe11 src/test/python/apache/aurora/client/api/test_updater.py e7eb1e783baaabe9437e55ecb848f6a42de13dd7 src/test/python/apache/aurora/client/cli/test_command_hooks.py 7c6f70c7ef7534e9dd4986364331c67f6f7c36b1 src/test/python/apache/aurora/client/cli/test_create.py 875573e2ee534ea50da71fac2174ecc8877a714d
Re: Review Request 20950: Moving kill wait to the client (Part 1: client changes)
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/20950/ --- (Updated May 1, 2014, 10:21 p.m.) Review request for Aurora, Mark Chu-Carroll and Brian Wickman. Changes --- CR comments. Bugs: AURORA-370 https://issues.apache.org/jira/browse/AURORA-370 Repository: aurora Description --- Migration plan: 1. Prepare client to poll for the status of tasks being killed. This is mostly a noop at this point while the scheduler wait with a shorter timeout is still in place. 2. Cut off scheduler killTasks wait thus relying on the client poll (coming next). This RB addresses client changes: - Repurposing JobMonitor to poll for all job tasks regardless of their arrival time. - Introducing a timeout cap into JobMonitor to interrupt wait upon reaching the max_poll_interval. - Setting the JobMonitor max_poll_interval to be higher than the scheduler killTasks backoff to ensure smooth migration. - Added wait_util calls into kill/killall/updater calls. Once this ticket is closed, the next step would be to make kill/killall truly async: AURORA-371 Diffs (updated) - src/main/python/apache/aurora/client/api/BUILD 32097d220a6f8a58555260ce081ace0c58b9fa99 src/main/python/apache/aurora/client/api/job_monitor.py b694ef62ba708c4b67c5cc931b0edfd93702027f src/main/python/apache/aurora/client/api/updater.py 0acf45034db124ed369d0bcb8095fe5545605eed src/main/python/apache/aurora/client/cli/jobs.py 782b34800def55c54b67ca5f13da66c4997a0777 src/main/python/apache/aurora/client/cli/task.py 62747ed3e3f5f6ecd9b9c46f6f175c634c068358 src/main/python/apache/aurora/client/commands/core.py 39190e06f3eac1ab4dabf48055418bc383be14ed src/test/python/apache/aurora/client/api/BUILD dd9b79764db0bbc3a11c51051e43b93a9b5d370a src/test/python/apache/aurora/client/api/test_job_monitor.py 32609e4e97afc7b8b164f3db438c60efd0ebbe11 src/test/python/apache/aurora/client/api/test_updater.py e7eb1e783baaabe9437e55ecb848f6a42de13dd7 src/test/python/apache/aurora/client/cli/test_command_hooks.py 7c6f70c7ef7534e9dd4986364331c67f6f7c36b1 src/test/python/apache/aurora/client/cli/test_create.py 875573e2ee534ea50da71fac2174ecc8877a714d src/test/python/apache/aurora/client/cli/test_kill.py cf5df648206e0cc234d603c1fd56dc7ab1311fa9 src/test/python/apache/aurora/client/cli/test_plugins.py 2dab749d1dcba99a1e3c792c8f26ee86441673d1 src/test/python/apache/aurora/client/cli/test_update.py cf077e821ebe32104b3e1345014f5c7b07c44c34 src/test/python/apache/aurora/client/cli/util.py 2225ab071dff72be547c43f66ffdc890171c3288 src/test/python/apache/aurora/client/commands/test_create.py e0ecb523db4ce3b1901adcc96a2fd5ce8184f621 src/test/python/apache/aurora/client/commands/test_kill.py db820a543b0ea11e5a40cc2a00209aa8fa6186c9 src/test/python/apache/aurora/client/commands/test_update.py 6e145db9ff8213c8f098b53b9a9668532d664249 Diff: https://reviews.apache.org/r/20950/diff/ Testing --- ./pants src/test/python:all Thanks, Maxim Khutornenko
Re: Review Request 20950: Moving kill wait to the client (Part 1: client changes)
On May 1, 2014, 8:59 p.m., Brian Wickman wrote: src/main/python/apache/aurora/client/api/job_monitor.py, line 81 https://reviews.apache.org/r/20950/diff/2/?file=572973#file572973line81 = Maxim Khutornenko wrote: It's safe as is but since Mark asked about this too changing it for better readability. it's not safe. you assume that min interval and max interval are powers of 2 of each other. plus then you run into python LOLisms like this mba=aurora=; python Python 2.6.9 (unknown, Apr 15 2014, 11:11:58) [GCC 4.2.1 (Based on Apple Inc. build 5658) (LLVM build 2336.11.00)] on darwin Type help, copyright, credits or license for more information. 1.1 1.1001 3 * 1.1 == 3.3 False - Brian --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/20950/#review41947 --- On May 1, 2014, 10:21 p.m., Maxim Khutornenko wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/20950/ --- (Updated May 1, 2014, 10:21 p.m.) Review request for Aurora, Mark Chu-Carroll and Brian Wickman. Bugs: AURORA-370 https://issues.apache.org/jira/browse/AURORA-370 Repository: aurora Description --- Migration plan: 1. Prepare client to poll for the status of tasks being killed. This is mostly a noop at this point while the scheduler wait with a shorter timeout is still in place. 2. Cut off scheduler killTasks wait thus relying on the client poll (coming next). This RB addresses client changes: - Repurposing JobMonitor to poll for all job tasks regardless of their arrival time. - Introducing a timeout cap into JobMonitor to interrupt wait upon reaching the max_poll_interval. - Setting the JobMonitor max_poll_interval to be higher than the scheduler killTasks backoff to ensure smooth migration. - Added wait_util calls into kill/killall/updater calls. Once this ticket is closed, the next step would be to make kill/killall truly async: AURORA-371 Diffs - src/main/python/apache/aurora/client/api/BUILD 32097d220a6f8a58555260ce081ace0c58b9fa99 src/main/python/apache/aurora/client/api/job_monitor.py b694ef62ba708c4b67c5cc931b0edfd93702027f src/main/python/apache/aurora/client/api/updater.py 0acf45034db124ed369d0bcb8095fe5545605eed src/main/python/apache/aurora/client/cli/jobs.py 782b34800def55c54b67ca5f13da66c4997a0777 src/main/python/apache/aurora/client/cli/task.py 62747ed3e3f5f6ecd9b9c46f6f175c634c068358 src/main/python/apache/aurora/client/commands/core.py 39190e06f3eac1ab4dabf48055418bc383be14ed src/test/python/apache/aurora/client/api/BUILD dd9b79764db0bbc3a11c51051e43b93a9b5d370a src/test/python/apache/aurora/client/api/test_job_monitor.py 32609e4e97afc7b8b164f3db438c60efd0ebbe11 src/test/python/apache/aurora/client/api/test_updater.py e7eb1e783baaabe9437e55ecb848f6a42de13dd7 src/test/python/apache/aurora/client/cli/test_command_hooks.py 7c6f70c7ef7534e9dd4986364331c67f6f7c36b1 src/test/python/apache/aurora/client/cli/test_create.py 875573e2ee534ea50da71fac2174ecc8877a714d src/test/python/apache/aurora/client/cli/test_kill.py cf5df648206e0cc234d603c1fd56dc7ab1311fa9 src/test/python/apache/aurora/client/cli/test_plugins.py 2dab749d1dcba99a1e3c792c8f26ee86441673d1 src/test/python/apache/aurora/client/cli/test_update.py cf077e821ebe32104b3e1345014f5c7b07c44c34 src/test/python/apache/aurora/client/cli/util.py 2225ab071dff72be547c43f66ffdc890171c3288 src/test/python/apache/aurora/client/commands/test_create.py e0ecb523db4ce3b1901adcc96a2fd5ce8184f621 src/test/python/apache/aurora/client/commands/test_kill.py db820a543b0ea11e5a40cc2a00209aa8fa6186c9 src/test/python/apache/aurora/client/commands/test_update.py 6e145db9ff8213c8f098b53b9a9668532d664249 Diff: https://reviews.apache.org/r/20950/diff/ Testing --- ./pants src/test/python:all Thanks, Maxim Khutornenko