-
Notifications
You must be signed in to change notification settings - Fork 101
Lambda toobig seatbelts #172
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
This brings up the question: Should we download the runtime before the user data/func pickles? |
pywren/wrenhandler.py
Outdated
@@ -32,6 +32,8 @@ | |||
|
|||
PROCESS_STDOUT_SLEEP_SECS = 2 | |||
|
|||
TMP_MIN_FREE_SPACE_BYTES = 10000000 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit - can we add a comment above this "Leave at least 10MB free space" or something like that
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This could be 1e8
instead?
pywren/wrenhandler.py
Outdated
condatar.extractall(runtime_etag_dir) | ||
try: | ||
condatar.extractall(runtime_etag_dir) | ||
except: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is there a particular exception that indicates the tarball was too big ? I guess there could be other errors like corrupt runtimes etc.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good question, I will dig further!
|
||
func_key_size = get_key_size(s3_client, s3_bucket, func_key) | ||
|
||
free_disk_bytes = free_disk_space("/tmp") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
is /tmp
hardcoded elsewhere as well or is there some variable we can use here cc @apengwin
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
/tmp
is hardcoded in current wrenhandler. The portability PR abstracts this out to a generic TEMP
variable that's set at the beginning, depending on what OS is running
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It is actually hardcoded in the runtimes as well. Unfortunately there are some absolute path dependencies that continue to be a problem in the way conda is packaged.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks. One last question: is the catch all except at the end to handle python3 or is to handle other unforeseen exceptions ?
So remember right now we only support running under the python2 runtime for the handler; it's for all the exceptions that I wouldn't otherwise catch, to make sure we have a way to clean up the extracted runtime. |
Ah got it. I forgot the handler was python2 specific. Sounds good. |
This is a series of fixes to handle:
if the runtime is too big, communicate to the user and execute proper cleanup on the worker so the worker isn't forever broken. (issue Too big of runtime #105)
If downloading the func or the data is too large for the remaining space, error as well.