Server shared context - Githubissues

Ogeon commented 10 years ago

Thank you for your contribution, but I'm still reluctant to include a global shared state like this because of these problems:

The state will be copied each time the server receives a connection and the copy is dropped when the connection is broken. This is not obvious and requires the state to work as a shared container.
The state may have to be mutable to be usable. This probably requires it to use mutexes and they, depending on where they are applied, may lock the whole server for short periods of time.
The is not shared between multiple instances in a cluster. This problem may be moved to the users of the library, but still.

These problems have to be considered before this can be implemented. I'm also planning a more generic implementation for the handlers, which will allow a handler local context. This, of course, doesn't exclude global context.

Ogeon commented 10 years ago

I should add that I'm also considering to add extensions/plug-ins to the server. A global shared state could be implemented as one of these extensions.

kvark commented 10 years ago

I'm not experienced in the domain, so please forgive me in advance for the nonsense.

Argumentation

The state will be copied each time the server receives a connection and the copy is dropped when the connection is broken.

False. Context is referenced as Arc<T>, where T does is not even bound by Clone or Copy.

The state may have to be mutable to be usable.

One can put a Cell or a RefCell inside the context for the mutable part, but it doesn't require us to change the way context is implemented. If some mutable state damages server performance, it's the user problem, not ours.

The is not shared between multiple instances in a cluster.

I assume each instance of a cluster will run its own server? In which case, yeah, it's the user problem, since user is the one who launches multiple servers. Our responsibility is the tasks within one server.

I'm also planning a more generic implementation for the handlers, which will allow a handler local context.

That's cool, but it gives you less than a global context. What I mean is: per-handler context can be merged in the global context, but not the other way around. Hence, global context gives you more abilities, and is easier to use and maintain.

I'm also considering to add extensions/plug-ins to the server.

Well, we can always remove this global context once an alternative is available. So far there is none.

Background

I was looking into a simple case: load the template(s) from an external file, and use it to generate the page. Doing that per-request is a waste, considering the IO load. Having the template(s) in the global context is a straightforward solution.

Ogeon commented 10 years ago

I see what you mean and I haven't rejected this pull request yet. The thing is that this is a domain where I haven't had time to figure out a good enough pattern and I prefer not to just mash something in and pray for its success (not that this is feature mashing, but still...). That's why I'm coming with these arguments and thank you for replying with some good points.

I do also have to take back a couple of things I wrote before: I missed the Arc while reading through the diff (I was in a bit of a hurry) and my idea regarding an extension for this is pointless, since it's a global state, by itself, and I can't see any more usecases to justify its existence.

Any way, I have been thinking some more about this during the day and your little background section confirms my recent thoughts; What Rustful needs is maybe not just a generic global state. What you describes is closer to a cache system and I think it would be good to have it built into the server in some way. This allows it to have some useful features like reloading when expired or freeing of unused resources or whatever (this requires more research).

A cache could then be accompanied by the handler local storage, defined by the user, and a database or some other more "safe, but slow" data storage, depending on what's necessary. The cache struct could be created outside the server and implement a Cache trait, which would allow the user to make a custom cache with extra features, like a generic storage, if necessary. I believe that would cover most of the cases.

I'm completely with you on the waste of resources part and I hope this made sense.

kvark commented 10 years ago

The cache idea that you described is interesting but quite vague. Could you please explain it in more detail? Perhaps, an example Cache trait would be handy. One thing that worries me a bit is that Cache is supposed to be mutable by definition. That makes it a rather complex solution for my case, where a simple immutable context would suffice.

Ogeon commented 10 years ago

I'm not a cache expert, so it's possible that I'm using the term in a slightly wrong way, but I'm thinking about a structure for shared resources. These resources are not necessary mutated by the handlers, but by the underlying system and only when they need to be updated (they may be able to expire).

Let's take your template example. We store the template in a file and this file will probably not be updated anytime soon. Therefore we can parse it and store it in RAM for later use to speed up the loading. This is a basic cached resource. It's shared between the handlers where it's used and it's not mutated after it's parsed (not counting insertion of content, but that's a separate problem for now).

Let's assume this resource may change sometime in the future. It can be spelling correction or other things that doesn't need the server to be recompiled. Then we can have a tiny routine which checks the date of the file before use to see if it has changed. It can then automatically reload the resource and return the new version. This mutation won't happen often, so it won't lock the whole system for any longer period.

All of this applies will also be usable for internally generated resources and it allows data to be lazily loaded, as well. This can be done with a trait which defines the basic functions. This is just a sketch:

trait CacheResource<T> {
    ///Returns preloaded resource.
    ///Reloads it if exprired (may have to take &mut self for this).
    ///It may also implement Deref for this, I think. That would be nice
    fn get(&self) -> T {
        if self.expired() {
            self.reload();
        }

        self.unwrap()
    }

    ///Just return resource
    fn unwrap() -> T;

    ///Check if resource has expired
    fn expired(&self) -> bool;

    ///Used to free unused resources (may have to take &mut self for this)
    fn free(&self);
}

This trait can be implemented by a resource container and stored in a cache struct. There could be one predefined for static storage and one which may reload resources. The cache struct itself can implement an other trait with a function for freeing resources. This needs some more planning, but I hope you get the point.

kvark commented 10 years ago

Thanks for the info. The Cache is starting to shape in my head, at least. I'll keep using the global context internally until you come up with a good cache implementation.

Ogeon commented 10 years ago

Thank you for discussing this with me, it did really help me to come to a more concrete idea, and I would have merged this pull request if it wasn't for those issues. I will do some experiments tomorrow and see if I can find a good implementation. I will probably build it in steps and begin with a simple load-and-store cache and add the more advanced features later.

Please post an issue or something if you find something helpful in your approach. I'm not totally impossible to deal with :)

Ogeon / rustful

Server shared context #4

Argumentation

Background