Skip to content

Conversation

@patrickbirch
Copy link
Collaborator

modified:   docs/install-js-lang.md
modified:   docs/js-lang-overview.md
modified:   docs/js-lang-privileges.md
modified:   docs/js-lang-procedures.md
modified:   docs/js-lang-troubleshoot.md
modified:   docs/js-lang-variables.md
modified:   docs/uninstall-js-lang.md

Copy link
Collaborator

@dlenev dlenev left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hello Patrick!

I think descriptions of hard and soft memory limits require adjustment.

Copy link
Collaborator

@dlenev dlenev left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hello Patrick!

A few more comments from me.


This variable controls the hard limit for JS routine memory allocation. When set to a non-zero value, the component calculates the hard limit as `js_lang.max_mem_size * js_lang.max_mem_size_hard_limit_factor` in bytes.

When a JS session's memory usage reaches the hard limit threshold, V8 triggers out-of-memory (OOM) handling, which terminates and crashes the entire mysqld process. This is different from the soft limit behavior, where V8 Isolate Termination stops only the JS execution and returns an error to the client.
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Perhaps also mention here the last chance GC before crashing process which you describe above?

	modified:   docs/install-js-lang.md
	modified:   docs/js-lang-overview.md
	modified:   docs/js-lang-privileges.md
	modified:   docs/js-lang-procedures.md
	modified:   docs/js-lang-troubleshoot.md
	modified:   docs/js-lang-variables.md
	modified:   docs/uninstall-js-lang.md
Copy link
Collaborator

@dlenev dlenev left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi Patrick!

After the latest changes the proposed PR looks OK to me.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants