search

typelevel / cats-effect

The pure asynchronous runtime for Scala
https://typelevel.org/cats-effect/
Apache License 2.0
2.03k stars 520 forks source link

WriterT[IO.. losing logs in parEvalMap hints at problem with Concurrent instance for WriterT ? #3764

Closed benhutchison closed 1 year ago

benhutchison commented 1 year ago

I'm seeing FS2's parEvalMap dropping logs emitted by WriterT[IO.., while the sequential cousin evalMap retains them, as does a "desugared" Writer using parEvalMap.

Since parEvalMap is written in terms of Concurrent this hints at a potential problem in the WriterT instance, although I was not able to spot a problem from a visual inspection.

import fs2.*
import cats.*
import cats.syntax.all.*
import cats.data.*
import cats.effect.*
import cats.effect.implicits.*

object ConcurrentWriterTTest extends IOApp.Simple:
  def tell(s: String): WriterT[IO, Chain[String], Unit] = WriterT.tell(Chain(s))

  val run = Stream(1)
    .parEvalMap(Int.MaxValue): _ =>
      tell("loglog")

    .compile.drain.written.flatMap(IO.println) //output: Chain()

object SequentialWriterTTest extends IOApp.Simple:
  def tell(s: String): WriterT[IO, Chain[String], Unit] = WriterT.tell(Chain(s))

  val run = Stream(1)
    .evalMap: _ =>
      tell("loglog")

    .compile.drain.written.flatMap(IO.println) //output: Chain(loglog)

object ConcurrentWriterDesugaredTest extends IOApp.Simple:
  def tell(s: String): IO[(Unit, Chain[String])] = IO.pure(() -> Chain(s))

  val run = Stream(1)
    .parEvalMap(Int.MaxValue): _ =>
      tell("loglog")

    .compile.toList.map(_.map(_._2)).flatMap(IO.println) //output: List(Chain(loglog))

Cats Effect 3.5.1 and FS2 3.7.0

armanbilge commented 1 year ago

See some related discussion in https://github.com/typelevel/fs2/issues/3246#issuecomment-1610356437.

tl;dr don't use monad transformer datatypes. You can replicate Writer-like semantics with Ref and you can even hide it behind the Tell abstraction in Cats MTL.

benhutchison commented 1 year ago

I want to respond on two levels..

  1. Cats Effect ships with instances for Concurrent for WriterT, and many others.

There's a few options that IMO are OK

There seems a tendency to bucket anything MT-related together, but it's more nuanced than that. There's a class of MTs like EitherT that are short-circuiting and seem to be problematic with the CE error channel, but WriterT isn't of that form.

When I inquired about this issue on TL Discord, Fabio's response actually pertained to a different situation, around error-handling, where WriterT drops logs on error (I actually believe the correct way to handle that is to raise an error of the form (E, L), ie the error E and any logs L accumulated up to the occurrence of the error).

Level two ,"Why are MTs worth bothering with anyway?" is more ideological and will come in a second comment..

benhutchison commented 1 year ago

Level 2: Why are MTs worth bothering with anyway?

I retain an interest in monad transformers not because I love them in themselves - actually they are a confusing pain - but because I retain a naive but as yet unshaken belief in simple functional programming.

By which I mean Runar's definition of functional programming, namely "programming with functions". Pure mappings whose only effect on the world is through their return type.

In the world of pure functions, state dependence manifests via a type signature like (A, S) => (B, S), logging via a signature like (A) => (B, L), errors via (A) => Either[E, B].

These kinds of pure signatures are, to me at least, intuitive, easy to reason about, and easy to test. Ease of testing remains one of the most compelling practical advantages of FP. I want to be able to write the computational pieces of my program using forms like those above. Monad Transformers let me write simple, pure code in the small, and yet lift it into a larger effectful program fairly gracefully.

It does not seem as nice to carry around a mutable Ref side-channel to store state or logs in. Pure functions become effectful. Type signatures start to "lie", in that they now omit some of the effects of a function. Asserting in a test becomes less simple.

SystemFw commented 1 year ago

In the world of pure functions, state dependence manifests via a type signature like (A, S) => (B, S), logging via a signature like (A) => (B, L), errors via (A) => Either[E, B].

I tried to make this point several times in the discussion you linked, but I think I've failed to convey it: in that world, you cannot use real concurrency nor native errors, so it's incompatible with IO. If you're willing to sacrifice that, i.e. having single-threaded simulated concurrency through yet another transformer (FreeT), and make sure that Exceptions are never thrown but always surfaced as an Either, then you might be able to achieve what you want, if not, this expectation is unrealistic.

Now I can't actually envisage why WriterT couldn't be supported for parEvalMap by monoidally combining the logs from the various concurrent fibers a different situation, around error-handling, where WriterT drops logs on error

I was actually trying to explain that it's a simpler case of the same issue: you need native mutable state, and WriterT gives you simulated state via function passing instead. In the error case, you need native mutable state because you are dealing with native exceptions, in the concurrency case, you need native mutable state to deal with concurrency. Not only that, but because fibers can fail and be interrupted, you also have to deal with the error scenario as well : where do you get the logs in (A, L) if the fiber gets interrupted? again, you need to store them in a Ref.

benhutchison commented 1 year ago

In the world of pure functions, state dependence manifests via a type signature like (A, S) => (B, S), logging via a signature like (A) => (B, L), errors via (A) => Either[E, B].

I tried to make this point several times in the discussion you linked, but I think I've failed to convey it: in that world, you cannot use real concurrency nor native errors, so it's incompatible with IO.

So a design goal I'm seeking for is that the individual pieces of (non-concurrent, non-exception-throwing) domain logic in my program are phrased as pure functions with simple signatures like above. What I think you're pointing out is that, as the program is composed & layered, as it scales up, as concurrency, 3rd party libs, io, and inevitably exceptions get involved, these simple forms aren't sufficient any more.

And parEvalMap is one of those boundaries, introducing concurrency.

At the moment I'm thinking about ways to transition from logging as (A) => (B, L) in the small, to using a Ref in the large.

benhutchison commented 1 year ago

Stepping back a bit though, I do think this issue and others that have appeared in the last year or so are unsettling.

If you read the blurb on CE3 it says something like:

And have an implementation of GenConcurrent[WriterT[IO, L, *]] that type checks and passes law checks, and yet it deadlocks, and drops logs, even when there are no errors.

That's troubling other people than me, right..?

Where's the gap..

armanbilge commented 1 year ago

At the moment I'm thinking about ways to transition from logging as (A) => (B, L) in the small, to using a Ref in the large.

I'm a broken record, but I highly recommend to use the Tell abstraction from Cats MTL.

https://typelevel.org/cats-mtl/mtl-classes/tell.html

It abstracts the main idea of WriterT. To satisfy the the Tell constraint, you can use either WriterT or Ref, but code written in terms of Tell doesn't need to know that.

These kinds of pure signatures are, to me at least, intuitive, easy to reason about, and easy to test. Ease of testing remains one of the most compelling practical advantages of FP.

IMHO a signature written in terms of Tell satisfies these criteria. If you feel that it does not, I would be interested to hear more about why you think that.

Is it that the instance is lawful, but not all essential qualities are described by the laws

When we say WriterT has a lawful implementation of GenConcurrent, we mean that it satisfies the laws defined by Cats Effect specifically for the GenConcurrent typeclass.

Notice that these laws have no specific notion of L or the "logging capability" of WriterT, because this is not a feature of the GenConcurrent typeclass. Therefore, the laws for that typeclass cannot make assertions specifically about L, because it does not know about it.

The "gap" is that you know about L and have expectations of its behavior that are not covered in the laws, since they lack specific knowledge of L.

Issues like this crop up in other places as well. For example consider https://github.com/typelevel/cats-effect/issues/3079. The original implementation of Resource#memoize was completely lawful, but its behavior was surprising because the GenConcurrent laws cannot make assertions about the behavior of Resource finalizers because they lack specific knowledge of that. In that case, we were able to replace it with another implementation that is lawful but has the expected behavior for finalizers.

and yet it deadlocks

Sorry, I missed this. Do you have a reference issue about WriterT deadlocking?

benhutchison commented 1 year ago

and yet it deadlocks

Sorry, I missed this. Do you have a reference issue about WriterT deadlocking?

No, Im sorry.. 😏 Loose talk on my part, I was referring to the EitherT deadlock you're well across..

armanbilge commented 1 year ago

Loose talk on my part, I was referring to the EitherT deadlock you're well across..

No problem, thanks for clarifying :) So I can answer that question now too, and it's essentially the same situation as WriterT.

Specifically, when you have EitherT[F, E, *] you actually have two error channels, one on F[_] and the other on EitherT itself (technically, on Either). However, the GenConcurrent laws do not know about EitherT's error channel. Therefore all lawful behavior is described with respect to errors raised on F[_].

Indeed, if you only ever use GenConcurrent[EitherT[F, E, *]].raiseError(...) I can guarantee you will never encounter those deadlock issues because that's precisely what the laws guarantee. The issue is when you work outside of the typeclass e.g. raising an error with EitherT.leftT[F, E](...). The typeclass does not know about this error channel and thus the laws make no guarantees about its behavior.

The tl;dr here is that laws can only make guarantees about the behavior of implementation of the typeclass, not "special" behaviors of the monad. So when you use the monad's "special" features, their behavior is not specified by the Cats Effect laws. This is why the behavior can be both lawful but "surprising".

armanbilge commented 1 year ago

Btw, I'm a broken record again, once again the solution is to use Cats MTL. Instead of EitherT use Handle.

https://typelevel.org/cats-mtl/mtl-classes/handle.html

The Handle constraint can then be satisfied with EitherT or directly with IO as demonstrated in https://github.com/typelevel/cats-mtl/pull/489. Most importantly, when you use the IO-based implementation you will not experience deadlocks, because the implementation in that PR uses a single error channel whose behavior is fully specified by the laws.

SystemFw commented 1 year ago

What I think you're pointing out is that, as the program is composed & layered, as it scales up, as concurrency, 3rd party libs, io, and inevitably exceptions get involved, these simple forms aren't sufficient any more.

Yes, think about this: the platform provides you with several primitives: 1) calling subroutines, 2) creating data, 3) mutating references, 4) creating system threads, 5) introducing synchronisation. These capabilities cannot be expressed in terms of each other, otherwise they won't be primitives.

In the world of pure functions, state dependence manifests via a type signature like (A, S) => (B, S), logging via a signature like (A) => (B, L), errors via (A) => Either[E, B].

With this, you're stating that basically you only want to use primitives 1) and 2) , and so by definition you won't be able to deal with primitives 3, 4 & 5, and you'd have to necessarily resort to a version of concurrency that uses FreeT without being able to use systems threads (basically no multi-core).

The ingenious trick or, depending on your perspective, the ugly hack that IO does is to let you access the primitives of the underlying platform whilst preserving a key characteristic of pure functions, i.e. referential transparency.

These kinds of pure signatures are, to me at least, intuitive, easy to reason about, and easy to test. Ease of testing remains one of the most compelling practical advantages of FP. I want to be able to write the computational pieces of my program using forms like those above. Monad Transformers let me write simple, pure code in the small, and yet lift it into a larger effectful program fairly gracefully. It does not seem as nice to carry around a mutable Ref side-channel to store state or logs in. Pure functions become effectful. Type signatures start to "lie", in that they now omit some of the effects of a function. Asserting in a test becomes less simple.

Now, let me challenge these statements a little bit.

Pure functions become effectful

It's actually hard to find a definition of purity that justifies this statement. Both StateT/ WriterT/ EitherT and IO are referentially transparent in the exact same way. Both force your code to be written in monadic style. The only difference is that StateT/WriterT/EitherT can be completely eliminated at any point in the program, whilst IO can only be eliminated in main, but I would like to understand what specific advantage you are getting out of that

easy to reason about

Well, in a way the very existence of this issue kinda disproves it but let's entertain the statement that all things being equal things like WriterT and EitherT are easier to reason about that IO and Ref. For one, I would say that the fact that WriterT[EitherT[Id, Err, *],L, A] and EitherT[WriterT[Id, L, *], Err, A] differ in the semantics of their state tends to be a point of confusion for a lot of people. But really, the issue is that all things are not equal, since they don't have the same power of IO, and once you replace Id with IO in those transformers, you start hitting the various issues we've been discussing

Type signatures start to "lie", in that they now omit some of the effects of a function

Now, this might be true if you compare:

def myFun(a: Int): WriterT[IO, Bar, String]

with

case class Foo(logs: Ref[IO, Bar]) {
  def myFun(a: Int): IO[String]
}

(although again, the first is fundamentally not as powerful as the second), but I don't think it's true for something like:

trait Logs[F[_]] {
  def log(b: Bar): F[Unit]
 } 
def myFun[F[_]: Logs : Concurrent](a: Int): F[String]

which you can implement over Kleisli[IO, Ref[IO, Bar], *] or by bundling a Ref in Logs[IO] or by using cats-mtl Tell[F, Bar]. This alternative has been proposed a few times and you haven't engaged with it, why don't you like it? (this is genuine curiosity, I personally don't value all this info in type signatures, but if you do, it seems to me you have plenty.)

easy to test. Ease of testing remains one of the most compelling practical advantages of FP. Asserting in a test becomes less simple.

I'll give you that doing:

def myFun(a: Int): WriterT[IO, Logs, String] = ...
myFun(3).run.flatMap { case (logs, value) =>
   assertions(logs) >> assertions(value)
 }

it's slightly easier than:

 def myFun[F[_]: Logs : Concurrent](a: Int): F[String] = ...

IO.ref(emptyBar).flatMap { state =>
   implicit val myLogs: Logs[IO] = Logs.fromRef(state)
   myFun(3).mproduct(state.get).flatMap { case (logs, value) =>
       assertions(logs) >> assertions(value)

but is it that much simpler? Especially when you consider that you can extend Logs directly with functions to help you test.

..and one that is kind of bad:

the instances remain out of inertia but don't work properly and are unsupported. When people point problems out, they are told "oh don't bother with them, they don't actually work".

I have to agree with you on this, however. @armanbilge has explained well why they can pass laws and still behave unexpectedly, but the end result of user confusion is all the same

armanbilge commented 1 year ago

Yes, just for the record I believe we should deprecate these instances. I expressed a similar opinion in https://github.com/typelevel/fs2/issues/3246#issuecomment-1612144626.