Still some issue with lambda serialisation. Will be looking into that.

final case class Equals(value: Any) extends Filter {
    val filterFunc: Any => Boolean = (item: Any) => value.equals(item)
    val operatorString: String = "="
    def valuesStrings: Set[Any] = Set(value)
  }

Is there a reason why filterFunc is a val? Looks like it could be a def also to avoid serialisation issues.

Adding @transient makes the tests pass, but will that be backwards compatible?
How do you deploy filo?

Making the val transient will break compatibility, but then the system hasn't fully stabilized yet anyhow, and so some issues are expected.

So what is your recommendation, @broneill ?
Feels like a bug that you send this lambda, that can be computed from value, over the wire.

In other concerns I have, it seems like for the Kryo serialisation we rely on the bytecode created by the Scala compiler.
We may have more backwards incompatible changes and the lambda issue is just one of them.

IIUC, we only test serialisation issues in the same build. Is there a way to test the migration?

[2020-08-27 19:25:56,107] WARN  akka.util.ManifestInfo [ManifestInfo(akka://akka-test)] - Detected possible incompatible versions on the classpath. Please note that a given Akka version MUST be the same across all modules of Akka that you are using, e.g. if you use [2.5.30] all other modules that are released together MUST be of the same version. Make sure you're using a compatible set of libraries. Possibly conflicting versions [2.5.22, 2.5.30] in libraries [akka-multi-node-testkit:2.5.22, akka-persistence:2.5.22, akka-protobuf:2.5.30, akka-coordination:2.5.22, akka-actor:2.5.30, akka-slf4j:2.5.22, akka-remote:2.5.30, akka-cluster:2.5.30, akka-stream:2.5.30, akka-cluster-tools:2.5.22]

This class path issue still has to be investigated.

It also seems like we are out of memory for sbt/test runs in the current build. Will have a look at this too.

Multiple akka versions on the class path is caused by a dependency added in
6522708 , specifically:

"org.sisioh"             %% "akka-cluster-custom-downing" % "0.0.21",

that depends on 2.5.30.

Created #874

Do you have any clues what fails?

From
[error] (cli / Test / test) sbt.TestsFailedException: Tests unsuccessful
I supposed it is cli, but the tests pass locally...

Found this

Uncaught error from thread [filo-cli-akka.remote.default-remote-dispatcher-7]: Could not initialize class scala.concurrent.Future$, shutting down JVM since 'akka.jvm-exit-on-fatal-error' is enabled for ActorSystem[filo-cli]
3017java.lang.NoClassDefFoundError: Could not initialize class scala.concurrent.Future$

Ok, confirmed that the failing test is FilodbCliSpec and possilbly problems with Kamon.

What I suggest is: remove Kamon start from FilodbClusterNode. Add it explicitly in each Filo entry point in:

1. CliMain
2. FiloServer

I'm not sure what about starting Kamon in a spark job...

That will mean that we don't start it in unit tests.

WDYT? @broneill @velvia

What I suggest is: remove Kamon start from FilodbClusterNode. Add it explicitly in each Filo entry point in:

1. CliMain
2. FiloServer

This should be fine until Kamon is initialized as the very first thing in the application by calling the Kamon.init() method. @szymonm Could you please make sure existing Config Load, Kamon Metrics and Tracing functionalities across FiloDB nodes are not broken after this move.

I'm not sure what about starting Kamon in a spark job...

@vishramachandran @sherali42 Please help here..

That will mean that we don't start it in unit tests.

WDYT? @broneill @velvia

@szymonm
Firstly, Thanks for the detailed PR and the pre-work for this. Excellent contribution in general!
I would like to not that since this is quite an invasive change which needs more detailed testing, the merge to develop needs to be put on hold for a couple of months possibly. We may hit a more quieter period towards end of the year when we'll get time for more regression testing. That is when we'll likely get this PR in. Hopefully this plan works for you - otherwise we may have to figure out other options - let's discuss.

BTW, we need kamon to be enabled in spark jobs. We do measure metrics that track progress of spark jobs. Thanks.

We don't need kamon in spark unit tests.

I'm already working on a fork, so I don't personally need this to be merged to develop. On the other hand I put considerable amount of work into figuring out all small fixes and would like to avoid conflict resolution as much as I can.

I understand that I don't have means of testing this change end to end. I consider the following parts of the change to be risky:

1. Scala version bump (due to binary incompatibility/possible performance changes around collections for instance/changes of dependencies).
2. Serialisation change in query in 5abd97c, which can make queries fail if you do a rolling rollout.
3. Kamon initialisation changes, because they are hard to verify on the unit tests level.
   I will definitely need help from someone on your side to use the E2E machinery and a careful production rollout, when the changes are deployed.

Other changes seem to be low risk and I consider them to be mostly fixing minor issues and tech debt (first commits of this PR). They don't depend on the risky changes and I will put them as a separate PRs.

Back to Kamon, I think the setup can be improved. Currently I see a lot of error logs during test run. After the move to 2.12 they fail FiloCliSpec. I believe that it's not a supported to call Kamon.init() in an object that is initialised inside a unit test. I think it's reasonable to avoid Kamon initialisation in the unit tests at all. This change shouldn't be risky.

More generally, I think we should avoid calling Kamon.init() in trait FilodbClusterNode that is mixed in by various application entry points. Instead I propose to call it in the entry points explicitly (there is not that many of them).

For spark I would just leave it as is (calling initialisation after mapPartitions or similar).

We may hit a more quieter period towards end of the year when we'll get time for more regression testing.

Can you share what is on your roadmap? I may be able to help.

We may hit a more quieter period towards end of the year when we'll get time for more regression testing.

How is that going? What are your plans regarding this PR?