Allow sources to fail individual results in a batch fetch

[stevenheidel]

Some RPC systems (like rest.li used at LinkedIn) allow a batch response to contain both successful and failed entries. For example, if I fetch the profiles for ids 1 and 2, it's possible that 1 could return a profile and 2 will return an UnauthorizedAccess. Visualized:

Map(
  1 -> Entity(Profile),
  2 -> Error(UnauthorizedAccess)
)

Currently I have only two choices to deal with this:

1. Remove all error entries from the Map. But then you lose all the error information which makes it difficult to debug why things are missing.
2. If I need access to the error, I can fail the entire fetch, but then you lose the profiles that were successfully returned.

This change fixes that by introducing the ability to explicitly define a Try in the resulting map. If the Try is a Failure, then that Clump will be failed. Otherwise it will behave as normal.

Added a .sourceTry method to create these types of sources. Everything else stays the same. Now if you use .sourceTry then the example above changes to the expected behaviour:
profileSource.get(1) will return a Clump containing the Profile.
profileSource.get(2) fails the Clump with an UnauthorizedAccess exception.

A good side effect of this is that .sourceSingle now handles exceptions much better as well. If one call to the single source throws an exception it only fails that Clump instead of failing everything in that "batch" fetch. I also took some time to refactor the single source creation into a parameterize method following the pattern of the other types of sources.

[stevenheidel]

In the future this means we can also add the ability to retry individual resources. So for example, if the batch fetch returns the following map:

Map(
  1 -> Entity(Profile),
  2 -> Error(DatabaseTimeout),
  3 -> Error(DatabaseTimeout)
)

We could send another fetch request with just keys 2 and 3.

[fwbrasil]

It sounds like a good change. Would the retry mechanism also be changed to retry only the failed keys?

[stevenheidel]

Well now there's two types of failures:
The whole request fails (as in the Future[Map[K,V]] is in a failed state, maybe some sort of network unavailable exception) or an individual value is marked as failed (marked as failed by the remote server).

I think there'd have to be separate retries for both.

In the case of sourceSingle it's sort of weird because individual entries and the entire request are always the same.

Do you think we need two different retry mechanisms then?

[fwbrasil]

Considering that the retry mechanism receives a partial function and we can configure it for each type of exception, I think it is generic enough to be used for both individual and batched failures. I don't see need for a separate mechanism.

[stevenheidel]

• Document .sourceTry method
• Update README with new functionality
• Add tests for individual failings in sourceSingle
• Add tests for individual failings in sourceTry
• Add tests for retry logic for individual entries failing in the map

[stevenheidel]

@fwbrasil @williamboxhall - added tests and docs. This is ready for a review now whenever you have time.

[fwbrasil]

I can't review in detail, but it looks good to me.

👍 if you need this asap

[williamboxhall]

should this be sourceTry in the docs here?

[stevenheidel]

You're right, I think I fixed it in the docs PR though, will double check.

On Mon, Nov 2, 2015, 16:17 William Boxhall [email protected] wrote:

In README.md
#109 (comment):

@@ -268,6 +268,20 @@ val usersSource = Clump.source(fetch )(.id)
val userClump = usersSource.get(id)

+### Map with Success and Failure ###
+
+Sometimes sources can mark individual entries as failed, even though the entire fetch function succeeded. For these sources, a function can be provided that maps from id to a Try object that can be marked as either Success or Failure.
+
+```scala
+def fetch(ids: List[Int]): Future[Map[Int, Try[User]]] = ...
+
+val usersSource = Clump.source(fetch _)

should this be sourceTry in the docs here?

—
Reply to this email directly or view it on GitHub
https://github.com/getclump/clump/pull/109/files#r43702956.

[stevenheidel]

Ya, I fixed it on master already 👍

[williamboxhall]

sweet