torrust-tracker
torrust-tracker copied to clipboard
torrust
Add a configurable memory limit to the torrent repository
Currently the torrent repository is not bound by an upper limit of torrents or memory usage. When the memory size of the torrent repository exceeds the amount of memory available on the host system, the tracker will restart.
To solve this problem, I want to add a configurable memory limit for the torrent repository. When the torrent repository is full, it should let new torrents replace old low priority torrents. For the first implementation I will base the priority on the time it was last accessed in an announce request.
Relevant discussions/issues
https://github.com/torrust/torrust-tracker/discussions/499 https://github.com/torrust/torrust-tracker/issues/566
Needs issues
https://github.com/torrust/torrust-tracker/issues/565
cc @josecelano @da2ce7
Hi, @WarmBeer I think an explicit limit is a nice feature but it would also be good to set a no-limit option so the tracker can consume as much memory as possible without panicking.
Hi, @WarmBeer I think an explicit limit is a nice feature but it would also be good to set a no-limit option so the tracker can consume as much memory as possible without panicking.
Will add a limit of 0 that will allow the torrent repository to grow until there is no more space available.
Hi, @WarmBeer I think an explicit limit is a nice feature but it would also be good to set a no-limit option so the tracker can consume as much memory as possible without panicking.
Will add a limit of
0that will allow the torrent repository to grow until there is no more space available.
@WarmBeer maybe -1 it's a better option.
ChatGPT:
When choosing a value to represent 'no limit' for a configuration option like memory_usage_limit in an application configuration file, it's essential to consider clarity, standard practices, and the potential for misinterpretation. Let's evaluate the options you've provided:
0: Typically, '0' implies no allocation or disabling the feature. In the context of memory usage, it might be misunderstood as not allowing the application to use any memory, which could confuse users.
-1: This is a common convention in many programming and configuration contexts to represent 'unlimited' or 'no limit'. It's widely recognized by developers and system administrators, making it a good choice for clarity and standard practice.
null: Using 'null' can be a good option, as it explicitly represents the absence of a value. However, it might require additional handling in the code to interpret 'null' as 'no limit' and not as an uninitialized or error state.
Empty (no value): An empty value might lead to confusion or be interpreted as a misconfiguration. It's less explicit than other options and might require additional documentation for users to understand its meaning.
"" (empty string): Similar to an empty value, an empty string can be ambiguous and might be interpreted as a configuration error. It's not a standard way to represent 'no limit' in a numerical context.
Considering these points, -1 seems to be the best choice. It's a widely recognized convention for 'no limit' in various programming and configuration contexts. It is explicit and less likely to be misinterpreted compared to the other options. However, it's important to clearly document this in the configuration file or accompanying documentation so that users understand that setting memory_usage_limit to -1 means there is no limit on memory usage.
Now that #690 has been merged, updating the torrent repository api to include memory tracking should be more simple.
I suggest closing this issue and opening a new proposal in discussions to implement this memory limit for all torrent repositories. When the issue was open, the intention was to add this memory limit only to the new DashMap implementation @mickvandijke was working on. However, I think we all agree we should implement it for all the repository implementations.
This is also related to @mickvandijke's proposal:
https://github.com/torrust/torrust-tracker/discussions/499
I guess that proposal is what you wanted to implement in this issue. And I think you implemented it in this PR. Rigth @mickvandijke?
I suggest closing this issue and opening a new proposal in discussions to implement this memory limit for all torrent repositories. When the issue was open, the intention was to add this memory limit only to the new DashMap implementation @mickvandijke was working on. However, I think we all agree we should implement it for all the repository implementations.
This is also related to @mickvandijke's proposal:
#499
I guess that proposal is what you wanted to implement in this issue. And I think you implemented it in this PR. Rigth @mickvandijke?
I've opened the new discussion: https://github.com/torrust/torrust-tracker/discussions/789