Dynamically set heap and direct memory limits. #49

aws-patlin · 2019-11-13T17:20:10Z

Description of changes:
The MaxDirectMemorySize parameter limits the amount of direct memory that the JVM can use. This has a direct effect of limiting the size of inference requests we can make, causing OutOfDirectMemoryErrors as soon as the 10MB limit is reached. This was temporarily raised during testing to 1GB.

After raising the direct memory limit, the Xmx parameter, which limits the max heap size, began causing OutOfMemoryErrors when testing batch transform jobs. After raising that limit as well, the errors were resolved.

The Xmx and MaxDirectMemorySize vmargs are now set based on the number of workers * the max payload size * a buffering factor of 1.2 + a base amount of 128mb, which is theoretically sufficient for the server when under full load (all workers occupied). Environment variables are not parsed for vmargs (MMS has not implemented this yet), so the current workaround is to write the values to the config.properties file before model startup.

Change were tested by running container tests and batch transform jobs with small (<5MB) and larger payloads (up to 20MB).

By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.

cbalioglu · 2019-11-13T18:22:34Z

docker/0.90-2/resources/mms/config.properties

@@ -1,4 +1,4 @@
-vmargs=-Xmx128m -XX:-UseLargePages -XX:+UseG1GC -XX:MaxMetaspaceSize=32M -XX:MaxDirectMemorySize=10m -XX:+ExitOnOutOfMemoryError
+vmargs=-Xmx128m -XX:-UseLargePages -XX:+UseG1GC -XX:MaxMetaspaceSize=32M -XX:MaxDirectMemorySize=1G -XX:+ExitOnOutOfMemoryError


Have you done any measurement? Why 1G and, say, not 100M?

ericangelokim · 2019-11-13T18:51:56Z

Please leave a comment explaining the TODO that should follow up

iyerr3 · 2019-11-13T23:55:27Z

Maybe this was discussed offline: is the parameter mandatory? Can MMS provide the functionality of detecting the right value per available memory?

aws-patlin · 2019-11-14T01:44:11Z

Maybe this was discussed offline: is the parameter mandatory? Can MMS provide the functionality of detecting the right value per available memory?

I think it becomes harder to control the memory behavior once we start hosting multiple containers on an endpoint, so it may be better to have a well defined limit. It also seems like removing the MaxDirectMemorySize parameter falls back to some other value, as I was seeing the same error around 100MB (could be tied to the Xmx parameter, which we also needed to increase).

aws-patlin · 2019-11-14T18:23:24Z

To confirm the default behavior when Xmx and MaxDirectMemorySize are not set:

Max heap size defaults to ¼ of the physical memory, up to 1GB if the server is 32-bit, or 32GB if the server is 64-bit. Ref
Max direct memory defaults to the value of the max heap size. Ref

In this case, it makes sense for us to be setting the limits to higher values depending on the instance type instead of relying on these defaults.

Code changed since last review.

edwardjkim · 2019-11-15T17:32:27Z

src/sagemaker_xgboost_container/serving.py

+                        + " -XX:MaxDirectMemorySize=" + os.environ["SAGEMAKER_MAX_DIRECT_MEMORY_SIZE"] + "\n")
+                g.write(f.read())
+    except Exception:
+        pass


Is the server going to start without config.properties? If not, is it ok to fail silently here?

I added this try-except because the path to the config.properties only exists on the actual container, which was causing the unit tests to fail. Is there a better way to do this?

I just read the code comments above the try-except block. If the plan is to remove this code in the near future, then I guess it's okay. Reading try: something, except Exception: pass made me think "it's okay to fail this block".

Yeah, ideally we want to have the config.properties as it was before and use the environment variables to get the vmargs. The issue is that the MMS team hasn't implemented environment variable parsing for vmargs, so this is a temporary solution until they release that feature.

Minor: instead of concat'ing strings, can you use format and use variable names to put values in the string for readability?

Also, I'm ok with this temporary solution but another solution worth thinking about would've been to take files and a dict and do overrides to create a new config file and its location.

edwardjkim · 2019-11-15T18:16:58Z

src/sagemaker_xgboost_container/serving.py

+                        + " -XX:MaxDirectMemorySize=" + os.environ["SAGEMAKER_MAX_DIRECT_MEMORY_SIZE"] + "\n")
+                g.write(f.read())
+    except Exception:
+        pass


I just read the code comments above the try-except block. If the plan is to remove this code in the near future, then I guess it's okay. Reading try: something, except Exception: pass made me think "it's okay to fail this block".

…umber of workers. Cap max_content_length to 20mb.

ericangelokim · 2019-11-17T03:03:58Z

src/sagemaker_xgboost_container/serving.py

+                        + " -XX:MaxDirectMemorySize=" + os.environ["SAGEMAKER_MAX_DIRECT_MEMORY_SIZE"] + "\n")
+                g.write(f.read())
+    except Exception:
+        pass


Minor: instead of concat'ing strings, can you use format and use variable names to put values in the string for readability?

Also, I'm ok with this temporary solution but another solution worth thinking about would've been to take files and a dict and do overrides to create a new config file and its location.

…osting. Use job queue size in max heap size calculation.

aws-patlin requested review from iyerr3 and ericangelokim November 13, 2019 17:20

cbalioglu reviewed Nov 13, 2019

View reviewed changes

cbalioglu previously approved these changes Nov 13, 2019

View reviewed changes

ericangelokim previously approved these changes Nov 13, 2019

View reviewed changes

aws-patlin force-pushed the inference branch 2 times, most recently from 58b544a to d57d2e6 Compare November 13, 2019 23:31

aws-patlin changed the title ~~Updated MaxDirectMemorySize from 10MB to 1GB.~~ Increased heap and direct memory limits to 1GB. Nov 14, 2019

aws-patlin force-pushed the inference branch from d57d2e6 to c21683b Compare November 15, 2019 00:32

aws-patlin changed the title ~~Increased heap and direct memory limits to 1GB.~~ Dynamically set heap and direct memory limits. Nov 15, 2019

aws-patlin force-pushed the inference branch from c21683b to f8606eb Compare November 15, 2019 00:46

aws-patlin requested review from cbalioglu and ericangelokim November 15, 2019 01:15

aws-patlin force-pushed the inference branch 2 times, most recently from dcf1b51 to 9d7eb10 Compare November 15, 2019 17:17

edwardjkim reviewed Nov 15, 2019

View reviewed changes

aws-patlin force-pushed the inference branch 2 times, most recently from dbb9276 to d6176db Compare November 15, 2019 17:52

edwardjkim approved these changes Nov 15, 2019

View reviewed changes

cbalioglu approved these changes Nov 15, 2019

View reviewed changes

aws-patlin force-pushed the inference branch from d6176db to 26a7ade Compare November 16, 2019 00:05

Dynamically set heap size and direct memory size based on the total n…

fe55154

…umber of workers. Cap max_content_length to 20mb.

aws-patlin force-pushed the inference branch from 26a7ade to fe55154 Compare November 16, 2019 00:08

ericangelokim approved these changes Nov 17, 2019

View reviewed changes

aws-patlin force-pushed the inference branch 2 times, most recently from addb00b to 27b54ef Compare November 20, 2019 01:40

Set job_queue_size to 2x num workers based on multi or single model h…

bb74d9c

…osting. Use job queue size in max heap size calculation.

aws-patlin force-pushed the inference branch from 27b54ef to bb74d9c Compare November 20, 2019 16:40

ericangelokim merged commit cc7c979 into master Nov 21, 2019

ericangelokim deleted the inference branch November 21, 2019 23:49

ericangelokim mentioned this pull request Nov 22, 2019

Migrate inference serer to MxNet Model Server aws/sagemaker-scikit-learn-container#25

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Dynamically set heap and direct memory limits. #49

Dynamically set heap and direct memory limits. #49

aws-patlin commented Nov 13, 2019 •

edited

cbalioglu Nov 13, 2019

ericangelokim commented Nov 13, 2019

iyerr3 commented Nov 13, 2019

aws-patlin commented Nov 14, 2019

aws-patlin commented Nov 14, 2019 •

edited

edwardjkim Nov 15, 2019

aws-patlin Nov 15, 2019

edwardjkim Nov 15, 2019

aws-patlin Nov 15, 2019

ericangelokim Nov 17, 2019

edwardjkim Nov 15, 2019

ericangelokim Nov 17, 2019

		@@ -1,4 +1,4 @@
		vmargs=-Xmx128m -XX:-UseLargePages -XX:+UseG1GC -XX:MaxMetaspaceSize=32M -XX:MaxDirectMemorySize=10m -XX:+ExitOnOutOfMemoryError
		vmargs=-Xmx128m -XX:-UseLargePages -XX:+UseG1GC -XX:MaxMetaspaceSize=32M -XX:MaxDirectMemorySize=1G -XX:+ExitOnOutOfMemoryError

Dynamically set heap and direct memory limits. #49

Dynamically set heap and direct memory limits. #49

Conversation

aws-patlin commented Nov 13, 2019 • edited

Choose a reason for hiding this comment

ericangelokim commented Nov 13, 2019

iyerr3 commented Nov 13, 2019

aws-patlin commented Nov 14, 2019

aws-patlin commented Nov 14, 2019 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

aws-patlin commented Nov 13, 2019 •

edited

aws-patlin commented Nov 14, 2019 •

edited