rudder icon indicating copy to clipboard operation
rudder copied to clipboard

Fixes #21825: Port code to ZIO2

Open fanf opened this issue 3 years ago • 5 comments

https://issues.rudder.io/issues/21825

As explained in the ticket, we have a very useful migration from where you copy&past things: https://zio.dev/guides/migrate/zio-2.x-migration-guide/

Main anoying/unintersting changes:

  • there is a bug in zio-json (https://github.com/zio/zio-json/issues/622) that forces us to not use codec (see ExpectedReports.scala)
  • lots of renaming, the most pervasive being .effect to .attempt. Good to understand, bad for future merge. For consistency, I also renamed IOResult.effect into IOResult.attemp
  • lots of others little renaming things like no more UIO.unit but ZIO.unit, RefM into Ref.Synchronized, all .xxxM methods into .xxxZIO methods, etc.
  • A bit more involving, no more Managed but ZIO.scope(...).flatMap(...) and no more bracket but ZIO.acquireReleaseWith
  • zio package as a System object, so we need to import zio.{System => _, _} to avoid conflict with java.lang.System
  • no more zio.duration._ package, the implicits for .seconds etc are in zio._
  • parallelism in traverse is not .foreachParN(...)(N) anymore but .foreach(...).withParallelism(N)
  • queue method getAll etc returns Chunk so it's simpler to use chunk elsewhere around them

More interesting:

  • untraced is not needed anymore: trace information are compile-time now and have almost not overhead at runtime
  • no need to pass clock around, they are now part of ZIO based effects always here. It's also much more easier to access it for sleeping (see for ex Scheduler.scala

The amazing

  • they corrected https://github.com/zio/zio/issues/1275 which means that WE DON'T HAVE TO CARE ANYMORE IF THINGS BLOCK! ZIO will try to do everything non-blocking and migrate the blocking effects in a dedicated threadpool. We can advise it, but it learn by itself what methods block and what don't! So we gain:
  • no more deadlock due to blocking effects that we didn't know about (rare case now for us, since by default, we told that everything was blocking -see https://github.com/Normation/rudder/pull/4507/files#diff-d4590d1d8c70916376ea07260ff5f905f68f324e53b5298c5e9038fbe913fc15R95)
  • no more special case for hotpoint, like LDAPConnectionProvider.scala
  • hopefully more consistant performance after a warm-up

And finally, the rewrite of CronParser.scala was kindly provided on ZIO chan.

We were lucky to not be using the IoC/Has framework which changed A LOT. Streams, too. All in all, the port is rather simple and for the hardest part, the community was extremelly helpfull.

fanf avatar Sep 22 '22 19:09 fanf

PR updated with a new commit

fanf avatar Sep 22 '22 19:09 fanf

PR updated with a new commit

fanf avatar Sep 22 '22 19:09 fanf

PR updated with a new commit

fanf avatar Sep 22 '22 20:09 fanf

PR updated with a new commit

fanf avatar Sep 22 '22 21:09 fanf

Tested on the load platform. No obvious perf improvment or degradation; policy generation seems ~5% faster, dynamic group computation time has less variability

ncharles avatar Sep 23 '22 15:09 ncharles

Tested on the load platform. No obvious perf improvment or degradation; policy generation seems ~5% faster, dynamic group computation time has less variability

amazing! (it is, because I removed all the hand-optimisation of effect/effectBlocking cherry picking, especially the one on LDAPConnection without which we were able to run rudder on the load plateform)

fanf avatar Sep 26 '22 10:09 fanf

PR updated with a new commit

fanf avatar Sep 26 '22 10:09 fanf

PR rebased

fanf avatar Oct 07 '22 16:10 fanf

PR rebased

fanf avatar Oct 07 '22 16:10 fanf

OK, merging this PR