Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[RFC 0159] General purpose allocator module #159

Closed

Conversation

lucasew
Copy link

@lucasew lucasew commented Aug 11, 2023

Summary

This RFC is a rescope of #151.

This RFC defines a function that returns a customized NixOS-like module that other modules can use to ask for allocated values and the module will check for invalid or undefined values or conflicts and suggest a value that solves the issue.

Rendered

@kevincox kevincox added the status: open for nominations Open for shepherding team nominations label Aug 23, 2023
@kevincox
Copy link
Contributor

This RFC is now open for shepherd nominations.

@tomberek
Copy link
Contributor

This RFC has not acquired enough shepherds. This typically shows lack of interest from the community. In order to progress a full shepherd team is required. Consider trying to raise interest by posting in Discourse, talking in Matrix or reaching out to people that you know.

If not enough shepherds can be found in the next month we will close this RFC until we can find enough interested participants. The PR can be reopened at any time if more shepherd nominations are made.

See more info on the Nix RFC process here

Copy link

@bew bew left a comment

Choose a reason for hiding this comment

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

Reading a bit more the discussion in the original RFC #151 and re-reading this RFC, it looks like I mis-understood the direction this is taking.

Reading your description in #151 (comment) :

It only accepts your config if all the ports are defined and also:

  • Checks for invalid ports (that don't pass the validation)
  • Checks for missing values, and
  • Checks for conflicts (conflict == same toString conversion, customizable)

And for those that don't comply, it suggests which change you can do right now to solve the issue by suggesting the next possible value to the key with issues.

It's not dynamic / smart to find unused values, the system just check the values are valid (and suggest a config change if they conflict).

🤔 Can we really call this an allocator? It's more a checker with this design.

I'm sorry, but I don't really see how this is a better idea than what was being discussed in #151 👀
(maybe you can explain?)


I'd be very interested in a general purpose allocator like the title suggests, that would be capable of doing what was being discussed in #151.

👉 Are you willing to go this route @lucasew ? or do you prefer to keep this RFC on the idea of a kind of 'general-space-checker' ?


This is an example of a port allocator using the function, plus a usage example:

```nix
Copy link

Choose a reason for hiding this comment

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

Hello!

I don't really understand this example :/

I don't really see where the magic happen here, what does the mkAllocationModule actually do / how is it useful in the example below.
From a birds eye view I see you set an option and use it just below, but I don't see a demonstration of the automatic port allocation the module seems to feature. Or am I reading it wrong?

I think the example should be more developed, and explicitly show and explain how it can be useful to have a system like this.


I like the initial description, it looks like a great idea!
But as a generic reader I think the example should prove to me and convince me of the idea if I had any doubts!

Copy link
Author

Choose a reason for hiding this comment

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

This implementation suggests a value if it's missing or invalid. If all values are existent, valid and no conflicts found then it accepts.

Otherwise it fails and suggests a valid value in the scope. In this case, a port.

The first implementation didn't have this "confirmation" so when you added another service, as attrsets are sorted in dictionary order, some services were restarted only because of the port changed, because that name ordering shift.

This way this problem do not happen.

# Summary
[summary]: #summary

A function that generates a suggestion based item allocator module.
Copy link

Choose a reason for hiding this comment

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

what is a suggestion in a deterministic system?
the word suggestion makes me think of uncertainty (might be just me though 🤷)

Maybe the summary here could describe the overall target, what this system would help achieve

Here you mention a function, I think the function in question is an implementation detail, the goal of this RFC is not to have a function!
👉 The goal of this RFC (as I see it) is to have a way to automatically compute values in a constrained space, so we don't have to precisely think of their precise values, and we can have high-level checks applied on top to assert invariants (e.g. uniqueness of ports) if necessary.

Copy link
Author

Choose a reason for hiding this comment

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

Suggestion = next available value in the value space

Ex: next port that isn't being used

Copy link

@bew bew Sep 20, 2023

Choose a reason for hiding this comment

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

Yeah I'm sorry, I mis-understood the subject of your RFC, I initially thought you wanted to compute all values, but instead want a kind of linter that can suggest config changes (very different!)

I think it should be described more explicitly to avoid this mis-understanding


One of these spaces, for example, is the port space, like, non administrative ports
for servers (1025..49151). You will probably put a reverse proxy in front of it
so even the stability of the port number is not so important.
Copy link

Choose a reason for hiding this comment

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

I think it would be helpful to also think about another space than ports to help design a general purpose system (otherwise can we say it is general if there is currently only one use case?).

Copy link
Author

Choose a reason for hiding this comment

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

That's precisely what I am blocked now.

How can I better generalize the abstraction.

so even the stability of the port number is not so important.

# Detailed design
[design]: #detailed-design
Copy link

Choose a reason for hiding this comment

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

In #151 there was a concern about stable value allocation even if we add/remove usage of the allocator:
#151 (comment)

Do you solve this problem? If yes, how?

I think this problem would benefit from some development in the RFC

@kevincox made a proposal for a design here: #151 (comment)
I think there are interesting things to use here, and would benefit from being included in this RFC if it wants to be generic, or at least offer a way to implement these ideas!

Copy link
Author

Choose a reason for hiding this comment

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

That's why it now suggests a value that you have to explicitly set later for it to accept.

valueKey = "port";
valueType = types.port;
cfg = config.networking.ports;
description = "Build time port allocations for services that are only used internally";
Copy link

Choose a reason for hiding this comment

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

(nit)
I initially read this as "Build (time port allocations) for services"
instead of "(Build time) (port allocations) for services"
and not understanding what it was ^^

Using build-time as a single word is easier to read I think

Suggested change
description = "Build time port allocations for services that are only used internally";
description = "Build-time port allocations for services that are only used internally";

so even the stability of the port number is not so important.

# Detailed design
[design]: #detailed-design
Copy link

Choose a reason for hiding this comment

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

In ports space it might be interesting to have sub-spaces (ranges of allocation?), where a some services could be associated with a range of possible ports. For example my service could have a public API on port 7000 and a more restricted API on port 7001. Both are related to the same service and could be grouped together.

These sub-spaces would ultimately be flattened out and checked like any other values in that space, but would allow grouping of related values.

Copy link
Author

Choose a reason for hiding this comment

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

@edolstra
Copy link
Member

edolstra commented Nov 1, 2023

This RFC is being closed due to lack interest. If enough shepherds are found this issue can be reopened. If you don't have permission to reopen please open an issue for the NixOS RFC Steering Committee linking to this PR.

@KiaraGrouwstra
Copy link

KiaraGrouwstra commented May 24, 2024

nixd's recently added options completion may fill a similar need

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
status: open for nominations Open for shepherding team nominations
Projects
None yet
6 participants