How to solve the N+1 problem? about async-graphql HOT 29 CLOSED

Yes, you store it in the context, datalaoder have an example about it w/ async_graphql https://github.com/cksac/dataloader-rs/blob/master/examples/async_graphql.rs

from async-graphql.

I see, your cache is only valid for a single query, right?

yes!

from async-graphql.

@krevativ Here's an example to answer your second question.

https://github.com/async-graphql/examples/tree/master/actix-web/token-from-header

from async-graphql.

I'm using a wrapper struct that contains all loaders which is created per request, this is my current code:

use bson::oid::ObjectId;
use std::any::{Any, TypeId};
use std::collections::HashMap;
use std::sync::Arc;
use tokio::sync::Mutex;

use crate::api::{ApiModel, ApiResult};

mod model;
use model::ModelLoader;

type AnyLoader = Box<dyn Any + Send + Sync>;

#[derive(Clone)]
pub struct Loaders {
    loaders: Arc<Mutex<HashMap<TypeId, AnyLoader>>>,
}

impl Loaders {
    pub fn new() -> Self {
        Loaders {
            loaders: Arc::new(Mutex::new(HashMap::new())),
        }
    }

    pub async fn load<T>(&self, key: ObjectId) -> ApiResult<Arc<T>>
    where
        T: ApiModel + 'static,
    {
        let loader = {
            let mut map = self.loaders.lock().await;

            map.entry(TypeId::of::<T>())
                .or_insert_with(|| Box::new(ModelLoader::<T>::new(ModelBatcher)))
                .downcast_ref::<ModelLoader<T>>()
                .unwrap()
                .clone()
        };

        loader.load(key).await
    }

    pub async fn clear_all(&self) {
        let mut loaders = self.loaders.lock().await;
        loaders.clear();
    }
}

(ApiModel, ApiResult, and ModelLoader being specific to my project)

With this wrapper, loaders are created on demand whenever a field needs it.

from async-graphql.

About the N+1 problem, I'm using dataloader with async_graphql for some weeks now and so far it works well

from async-graphql.

@sunli829

Thanks. I use the tide framework and your answer led me to:

https://github.com/async-graphql/examples/blob/master/tide/token-from-header/src/main.rs

from async-graphql.

@IcanDivideBy0 I've edited my comment. It was the outdated uuid package version.

from async-graphql.

About the N+1 problem, I'm using dataloader with async_graphql for some weeks now and so far it works well

I saw it already but I can't figure out where to store the dataloaders? Do you store them in the context/state? Could you show me a small example?

from async-graphql.

Yes, but currently tide does not support websocket, so subscriptions are not supported. 😁

from async-graphql.

@sunli829

I still have a lot to learn about GraphQL and rust, so I can wait :) But I'm eagerly waiting for subscriptions. This will be awesome! If you need some help, just point me to the problem. Maybe I can help.

Have you some thoughts on live queries too?

For now, I only have some problems with the scalar types. This is strange, but when I try to use the ID type, I can't convert it from an Uuid. Rust simply doesn't see that From<Uuid> is implemented. I want to look more deeply into the problem later on. Maybe my cargo workspace is fault? Don't know for now.

I also think that all scalar types should derive Serialize and Deserialize traits, so you can use it with serde. What do you think?

from async-graphql.

Have you some thoughts on live queries too?

1. Live queries is @live directive?

For now, I only have some problems with the scalar types. This is strange, but when I try to use the ID type, I can't convert it from an Uuid. Rust simply doesn't see that From is implemented. I want to look more deeply into the problem later on. Maybe my cargo workspace is fault? Don't know for now.

2. You can convert Uuid to ID directly by calling Into::into.

I also think that all scalar types should derive Serialize and Deserialize traits, so you can use it with serde. What do you think?

3. Most scalars support serde, and I'll add those that don't as soon as possible, thanks for your suggestions.😁

from async-graphql.

1. Live queries is `@live` directive?

Yes and the necessary transport layer. As far as I understand it, it means keeping the client-state on the server and having some pub-sub mechanics in place, to notify the client about the changes. I do understand that it's a huge task :) But it could be an interesting idea to explore sometime in the future.

Here is one talk on this topic: https://www.youtube.com/watch?v=g-asVW9JFPw

1. You can convert `Uuid` to `ID` directly by calling `Into::into`.

No I can't. Just tried it again and it fails. Here is short version of my code:

#[async_graphql::SimpleObject(desc = "A User")]
#[derive(Debug, Clone)]
struct User {
    id: async_graphql::ID,
}

impl User {
    fn random() -> Self {
        User {
            id: Uuid::new_v4().into(),
        }
    }
}

Nothing magical. And I've also tried ID::from(Uuid::new_v4()). And what I get is:

the trait bound `async_graphql::scalars::id::ID: std::convert::From<uuid::Uuid>` is not satisfied
...
help: the following implementations were found:
             <async_graphql::scalars::id::ID as std::convert::From<&'a str>>
             <async_graphql::scalars::id::ID as std::convert::From<bson::oid::ObjectId>>
             <async_graphql::scalars::id::ID as std::convert::From<std::string::String>>
             <async_graphql::scalars::id::ID as std::convert::From<usize>>
             <async_graphql::scalars::id::ID as std::convert::From<uuid::Uuid>>

Edit: Ups. The problem was that I was using 0.7 version of the uuid package (because I've tried juniper first and it only supports this version). After update to the latest version everything works fine.

from async-graphql.

@krevativ which version of async_graphql are you using ?

from async-graphql.

In fact, I have tried to support @defer, @stream and @LiVe, but since rust currently does not support specialization, I need to add some wapper types, which is very inconvenient to use, so I consider waiting for the specialization to stabilize before adding these features.😃

from async-graphql.

Rust really needs specialization. I miss this feature too. Ok it's probably better to wait then.

from async-graphql.

I have a followup questions on the N+1 problem and dataloader. So if I understand it correctly the current solution is to create all dataloaders for each request and provide them through async_graphql::query::QueryBuilder.data

My code for the tide framework looks something like this:

    app.at("/gql").post(|req: Request<State>| async move {
        let schema = req.state().schema.clone();

        async_graphql_tide::graphql(req, schema, |mut query_builder| {
            query_builder.data(GqlData {
                some_loader: Loader::new(SomeLoader),
            })
        })
        .await
    });

But isn't it wasteful to create all dataloards on each request? What if I don't need them for a specific query?

from async-graphql.

I'm not really sure this is necessary anyways, Loader creation is pretty cheap. I mostly do it to avoid having declare new loader every time I declare a new type in my api.

from async-graphql.

That's a great idea :) Thanks, now it makes much more sense.

from async-graphql.

@krevativ @IcanDivideBy0

Sorry, the documentation of async-graphql is not very clear about this piece. In fact, it is divided into two types of data, Schema data and Context data. Schema data is set by SchemaBuilder::data, and Context data is set by QueryBuilder::data, you can think of Schema data as global, and Context data is only valid for this query, and use Context::data to get them.

from async-graphql.

@IcanDivideBy0 could you please elaborate more on how your ModelLoader works? I try to implement it now and I do understand the basic concept. But I can't understand how does ModelLoader::::new(ModelBatcher) returns the corresponding loader? Loader<K, V, F> is much more generic. So, your ModelLoader must somehow infer the other generic parameters from T?

from async-graphql.

Here is an example: https://github.com/async-graphql/examples/tree/master/tide/dataloader

from async-graphql.

@sunli829 thanks. Great that there is a dataloader example now. But this is just a normal dataloader example that brings me back to my waste problem. I think it's wasteful to create all dataloaders for each request, just in case you will need them. This is maybe a cheap operation, but if you have a lot of dataloaders and a small incoming query, it is kind of out of proportion. Don't you think? I like the idea from @IcanDivideBy0 to create them on demand. But I try to understand now, how his solution returns the corresponding loader just from T?

from async-graphql.

@krevativ I use a ApiModel trait that is general to my whole application that anything I want to fetch from my DB need to implement. With such a trait, the loader can be generic too:

pub trait ApiModel: for<'a> Deserialize<'a> {
    const COLLECTION: &'static str;

    fn id(&self) -> &ObjectId;

    fn load_many(ids: &[ObjectId]) -> Vec<Self> {
        // query database using T::COLLECTION
        // then use serde to convert db result to Self
    }
}

// ---------------------

#[derive(Clone)]
pub struct ModelBatcher;

#[async_trait]
impl<T: ApiModel> BatchFn<ObjectId, T> for ModelBatcher {
    async fn load(&self, keys: &[ObjectId]) -> HashMap<ObjectId, T>
    where
        T: 'async_trait,
    {
        let res = T::load_many(keys)
            .drain(..)
            .map(|model| (model.id().clone(), model))
            .collect()
    }
}

pub type ModelLoader<T> = Loader<ObjectId, T, ModelBatcher>;

This is a bit simplified but the global idea is here

from async-graphql.

The corresponding dataloader should be created according to the data category. For example, if you have users table and posts table, then you should create two dataloaders, UsersDataLoader and PostsDataLoader, instead of creating DataLoader for the request.

from async-graphql.

@sunli829 dataloader have a layer of cache in its implementation, so in Javascript at least, it's usual to recreate all loaders per request. That's what we're doing here: one loader per model but @krevativ want to avoid re-creating all loaders on every request because most of them might not be used at all for this request.

from async-graphql.

@IcanDivideBy0
I just don't understand why it was recreated, wasn't the loader created at the beginning? 🤔

from async-graphql.

@sunli829 I have a struct Loaders which lives in QueryContext and holds all possible Loader.
By using dataloader cached Loader, we don't want this to be in SchemaContext because of the cache.

from async-graphql.

I see, your cache is only valid for a single query, right?

from async-graphql.

@krevativ @IcanDivideBy0

Sorry, the documentation of async-graphql is not very clear about this piece. In fact, it is divided into two types of data, Schema data and Context data. Schema data is set by SchemaBuilder::data, and Context data is set by QueryBuilder::data, you can think of Schema data as global, and Context data is only valid for this query, and use Context::data to get them.

@IcanDivideBy0 and @sunli829, I am trying to change the value of a field in the context from within a resolver. I also would like to set new values in the context from within a resolver. How can I do that?

from async-graphql.