nidomiro/kdataloader

A Kotlin implementation of dataloader

kdataloader's Issues

Implement a DataLoaderFactory

Verify "threadsafety" of `dispatch`

Verify that calling dispatch in parallel will start one dispatch-execution only.

SimpleStatisticsCollector throws ConcurrentModificationException

Sometimes with a huge data tree I'm getting this error:

java.util.ConcurrentModificationException: null
	at java.util.ArrayList$Itr.checkForComodification(ArrayList.java:1013) ~[?:?]
	at java.util.ArrayList$Itr.next(ArrayList.java:967) ~[?:?]
	at nidomiro.kdataloader.statistics.SimpleStatisticsCollector.createStatisticsSnapshot(SimpleStatisticsCollector.kt:80) ~[KDataLoader-jvm-0.3.0.jar:?]
	at nidomiro.kdataloader.statistics.SimpleStatisticsCollector$createStatisticsSnapshot$1.invokeSuspend(SimpleStatisticsCollector.kt) ~[KDataLoader-jvm-0.3.0.jar:?]
	at kotlin.coroutines.jvm.internal.BaseContinuationImpl.resumeWith(ContinuationImpl.kt:33) [kotlin-stdlib-1.5.0.jar:1.5.0-release-749 (1.5.0)]
	at kotlinx.coroutines.DispatchedTask.run(DispatchedTask.kt:106) [kotlinx-coroutines-core-jvm-1.4.3-native-mt.jar:?]
	at kotlinx.coroutines.scheduling.CoroutineScheduler.runSafely(CoroutineScheduler.kt:571) [kotlinx-coroutines-core-jvm-1.4.3-native-mt.jar:?]
	at kotlinx.coroutines.scheduling.CoroutineScheduler$Worker.executeTask(CoroutineScheduler.kt:750) [kotlinx-coroutines-core-jvm-1.4.3-native-mt.jar:?]
	at kotlinx.coroutines.scheduling.CoroutineScheduler$Worker.runWorker(CoroutineScheduler.kt:678) [kotlinx-coroutines-core-jvm-1.4.3-native-mt.jar:?]
	at kotlinx.coroutines.scheduling.CoroutineScheduler$Worker.run(CoroutineScheduler.kt:665) [kotlinx-coroutines-core-jvm-1.4.3-native-mt.jar:?]

This happens inside the KGraphQL when having multiple thousands of records and where it's using nested GraphQL fields.

Cache get's passed along from Factory

Each dataLoader constructed from the dataLoaderFactory should have it's own cache.

Here's a sample of a failing test:

@Test
fun `assure Factory doesn't pass cache`() = runBlockingWithTimeout {
    val dataLoaderFactory = dataLoaderFactory(
        { keys: List<Int> -> keys.map { ExecutionResult.Success(it.toString()) } }
    )

    val loader1 = dataLoaderFactory.constructNew()
    val loader2 = dataLoaderFactory.constructNew()

    loader1.loadAsync(2)
    loader2.loadAsync(2)

    assertThat(loader1.createStatisticsSnapshot().objectsRequested).isEqualTo(1)
    assertThat(loader2.createStatisticsSnapshot().objectsRequested).isEqualTo(1)

    assertThat(loader1.createStatisticsSnapshot().cacheHitCount).isEqualTo(0)
    assertThat(loader2.createStatisticsSnapshot().cacheHitCount).isEqualTo(0)

    loader2.loadAsync(2)
    assertThat(loader2.createStatisticsSnapshot().cacheHitCount).isEqualTo(1)
}

JCenter closing down

As JCenter is closing down, it would be great if you could move this project to MavenCentral or some other stable place.

KGraphQL(aPureBase/KGraphQL#134) has moved over to MavenCentral and users are reporting issues regarding KDataLoader not beeing resolved.

createStatisticsSnapshot not always reliable

As seen here: https://github.com/nidomiro/KDataLoader/commit/649109a3a710f26cdb19406e35206c849d7bbc60/checks?check_suite_id=410490317 Link somehow changed.
a Job failed once at DataLoaderStatisticsTest.kt:242.

I did also have this issue when creating my latest PR(#7), which I solved by doing a hack with using delay:
https://github.com/nidomiro/KDataLoader/blob/master/src/jvmTest/kotlin/nidomiro/kdataloader/DSLTest.kt#L161-L185

One solution could be these changes in SimpleDataLoaderImpl

statisticsScope = CoroutineScope(Dispatchers.Default)
// and add this:
private val statisticsJobs = mutableListOf<Deferred<Unit>>()

// ... and
return if (options.cacheEnabled) {
    options.cache.getOrCreate(key, block, { statisticsScope.launch { statisticsCollector.incCacheHitCount() } })
    // Changed to :
    options.cache.getOrCreate(key, block, { statisticsJobs.add(statisticsScope.async { statisticsCollector.incCacheHitCount(); Unit }) })
} else {
    block(key)
}

// ... and then
override suspend fun createStatisticsSnapshot(): DataLoaderStatistics {
    statisticsJobs.awaitAll()
    return statisticsCollector.createStatisticsSnapshot()
}

Dispatching with graphql-java

Has anybody used this successfully with graphql-java? The docs allude to how this is useful for GraphQL but don't give a concrete example of how to use it.

The trick with graphql-java is figuring out where to insert the dispatch calls. Ideally you want them to be roughly "when the queue is empty", like in Node.

The Java DataLoader has integration with graphql-java itself, but that integration is very constrictive — it only works if you invoke DataLoader directly from the synchronous part of the field fetcher (ie, you can't use two DataLoaders in series in one fetcher). graphql-java/graphql-java#1198 has the details.

Simplify usage by changing BatchLoader

suspend (ids: List<K>) -> List<ExecutionResult<R>>

to

suspend (ids: Set<K>) -> Map<K, ExecutionResult<R>>

Changing the BatchLoader like that would simplify the whole usage and removes the need to be careful about returning a sorted list.

Sample usage after this change:

@Test
fun `create a basic DataLoader`() = runBlockingWithTimeout {
    val dataLoader = dataLoader({ keys: Set<Int> ->
        // We return a map, so it doesn't matter what order we execute this
        keys.reversed().map {
            it to ExecutionResult.Success(it.toString())
        }.toMap()
    })


    val deferred1 = dataLoader.loadAsync(1)
    val deferred2 = dataLoader.loadAsync(1)
    val deferred3 = dataLoader.loadAsync(3)
    dataLoader.dispatch()
    assertThat(deferred1.await()).isEqualTo("1")
    assertThat(deferred2.await()).isEqualTo("1")
    assertThat(deferred3.await()).isEqualTo("3")
}

Build Sources.jar for Multiproject

Implement statistics into the DataLoader

Some real world examples?

Hi, currently it's super unclear how to use it with graphql-kotlin let say, or with graphql-java.

Like, I'm unsure when and where I can call dispatch in case of graphql-koltin to execute data loaders and to get results returned back to my resolvers.

Also, on the right side of your docs website, there are unused sections in the nav :) Remove them as it looks super unpolished.
Overall, the lack of examples and real-world scenarios make it unusable at least in the case of using graphql-kotlin. Maybe it's clearer how to do that with graphql-java, but I'm not super familiar as I'm using graphql-kotlin.

Thanks. I really hope for more examples. As for now I'm not even sure how to import Dataloader instance :D
As even in the example below

fun main(): Unit = runBlocking {
    val batchLoader: BatchLoader<Int, Int> = { keys -> keys.map { ExecutionResult.Success(it) } }
    val dataLoader = dataLoader(batchLoader)

    val value1 = dataLoader.loadAsync(1)
    val value2 = dataLoader.loadAsync(2)
    dataLoader.dispatch()

    println("1 -> ${value1.await()}")
    println("2 -> ${value2.await()}")
}

I can see where batchLoader is came from, but how is dataLoader has been created? :D Any class name, or, imports, dunno :D