Skip to content

Set default in memory timer limit to 1024#467

Merged
tillrohrmann merged 6 commits intorestatedev:mainfrom
tillrohrmann:issue#386
Jun 1, 2023
Merged

Set default in memory timer limit to 1024#467
tillrohrmann merged 6 commits intorestatedev:mainfrom
tillrohrmann:issue#386

Conversation

@tillrohrmann
Copy link
Copy Markdown
Contributor

This fixes #386.

@tillrohrmann
Copy link
Copy Markdown
Contributor Author

One thing I couldn't figure out is how to set the limit to None when using the environment variables. With the restate.yaml one can set num_timers_in_memory_limit: null and it will disable the limit. Maybe we need to provide a custom deserializer that can understand None.

@slinkydeveloper
Copy link
Copy Markdown
Contributor

slinkydeveloper commented May 30, 2023

@tillrohrmann IMO null is probably the right way, and perhaps the one that is more idiomatic in json, given there is no concept of Option and None

}

fn default_num_timers_in_memory_limit() -> usize {
1024
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I really need to build a macro for configuring those default values :(

@tillrohrmann
Copy link
Copy Markdown
Contributor Author

@tillrohrmann IMO null is probably the right way, and perhaps the one that is more idiomatic in json, given there is no concept of Option and None

null only works with the restate.yaml. When setting it via the env variable it leads to a failure where null string is not a valid usize :-(

@slinkydeveloper
Copy link
Copy Markdown
Contributor

slinkydeveloper commented May 30, 2023

When setting it via the env variable it leads to a failure where null string is not a valid usize

Perhaps just use export ENV_NAME so the value is empty? But yeah perhaps figment should accept null here.

@tillrohrmann
Copy link
Copy Markdown
Contributor Author

When setting it via the env variable it leads to a failure where null string is not a valid usize

Perhaps just use export ENV_NAME so the value is empty? But yeah perhaps figment should accept null here.

One cannot export an empty env variable as far as I know (at least it is not working in my env).

@slinkydeveloper
Copy link
Copy Markdown
Contributor

But an empty one yes?

!10755 ➜ export MY_ENV_NAME=""                                           
!10756 ➜ echo ${MY_ENV_NAME-nonexistent}                             

!10757 ➜ echo ${UNKNOWN-nonexistent}                          
nonexistent

@tillrohrmann
Copy link
Copy Markdown
Contributor Author

But an empty one yes?

!10755 ➜ export MY_ENV_NAME=""                                           
!10756 ➜ echo ${MY_ENV_NAME-nonexistent}                             

!10757 ➜ echo ${UNKNOWN-nonexistent}                          
nonexistent

Does not work either because an empty string won't be detected as an Empty value. Same error that string was found but usize was expected.

@slinkydeveloper
Copy link
Copy Markdown
Contributor

Does not work either because an empty string won't be detected as an Empty value. Same error that string was found but usize was expected.

Ah dammit, then open an issue about this and I'll take a look.

@tillrohrmann
Copy link
Copy Markdown
Contributor Author

Here is the ticket: #478

@tillrohrmann tillrohrmann merged commit 4694885 into restatedev:main Jun 1, 2023
@tillrohrmann tillrohrmann deleted the issue#386 branch June 1, 2023 07:36
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.

Set default limit for in memory timers

2 participants