Fixup total memory parsing logic #46
Conversation
| @@ -167,3 +162,22 @@ func (m MemoryCalculator) Execute() (map[string]string, error) { | |||
|
|
|||
| return map[string]string{"JAVA_TOOL_OPTIONS": strings.Join(values, " ")}, nil | |||
| } | |||
|
|
|||
| func parseMemInfo(s string) (int64, error) { | |||
| pattern := `MemAvailable:\s*(\d+)(.*)` | |||
There was a problem hiding this comment.
Can we always be sure that the amount is shown as kB?
My approach was that in case of a different unit it would be safer to not use it.
There was a problem hiding this comment.
https://github.com/paketo-buildpacks/libjvm/pull/46/files#diff-0ab20a99dcc0be1692ffca784255e81c58b78b03b66a6bfc65f77b6e82e3e16aR178-R182178 is now parsing the units explicitly. In practice the unit appears to always be kB but the documentation potentially leaves open the option for other units so I decided to go with the more complete/paranoid version while I was making changes.
There was a problem hiding this comment.
sorry totally missed the second match group.
| if mem, err := parseMemInfo(string(b)); err != nil { | ||
| m.Logger.Infof(`WARNING: failed to parse available memory from path %q: %s`, m.MemoryInfoPath, err) | ||
| } else { | ||
| totalMemory = mem |
There was a problem hiding this comment.
Should we log a short info "memory calculation is based on x amount of available memory"?
When reading the logs of a container it way be hard to understand why the jvm memory was calculated this way. Especially as it may be impossible to check what the available memory was at startup.
* injects correct default memory info filepath * allows specified prefixes when parsing memory.limit_in_bytes * parses units from available memory instead of only matching kB * no longer support nonstandard b and B suffixes when parsing memory size * replace parsing error cases with warning and continue to next fallback * formatting Signed-off-by: Emily Casey <ecasey@vmware.com>
2e29946 to
36564f4
Compare
Signed-off-by: Emily Casey ecasey@vmware.com
Please confirm the following: