Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

regression with (unreleased) 2.12.12-SNAPSHOT and akka-cluster 2.5.13 #12039

Closed
retronym opened this issue Jun 15, 2020 · 7 comments
Closed

regression with (unreleased) 2.12.12-SNAPSHOT and akka-cluster 2.5.13 #12039

retronym opened this issue Jun 15, 2020 · 7 comments
Assignees
Milestone

Comments

@retronym
Copy link
Member

akka.cluster.StreamRefMultiJvmNode3

akka > ++ 2.12.11=/Users/jz/code/scala/build/pack
akka > akka-cluster/multi-jvm:test

More details to follow as they emerge...

@retronym retronym added this to the 2.12.12 milestone Jun 15, 2020
@retronym retronym self-assigned this Jun 15, 2020
@retronym
Copy link
Member Author

retronym commented Jun 15, 2020

Testing with:

$ git fetch --tags
$ git co v2.5.13 # version from the end-user report
$ sbt
akka > ++ 2.12.11=/Users/jz/code/scala/build/pack
akka > akka-cluster/multi-jvm:testOnly akka.cluster.UnreachableNodeJoinsAgain

...
895] - Leader is moving node [akka.trttl.gremlin.tcp://UnreachableNodeJoinsAgainSpec@localhost:53897] to [Up]
[info] [JVM-1] [INFO] [06/15/2020 18:32:13.573] [UnreachableNodeJoinsAgainSpec-akka.actor.default-dispatcher-20] [akka.cluster.Cluster(akka://UnreachableNodeJoinsAgainSpec)] Cluster Node [akka.trttl.gremlin.tcp://UnreachableNodeJoinsAgainSpec@localhost:53895] - Leader is moving node [akka.trttl.gremlin.tcp://UnreachableNodeJoinsAgainSpec@localhost:53898] to [Up]
[info] [JVM-1] - must reach initial convergence (on node 'first', class akka.cluster.UnreachableNodeJoinsAgainMultiJvmNode1) *** FAILED *** (25 seconds, 245 milliseconds)
[info] [JVM-1]   Set(Joining) did not equal Set(Up) (MultiNodeClusterSpec.scala:309)
[info] [JVM-1]   org.scalatest.exceptions.TestFailedException:
[
...
[info] [JVM-4] - must reach initial convergence (on node 'fourth', class akka.cluster.UnreachableNodeJoinsAgainMultiJvmNode4) *** FAILED *** (25 seconds, 134 milliseconds)
[info] [JVM-1]   at scala.collection.immutable.List.foreach(List.scala:431)
[info] [JVM-4]   Set(Joining) did not equal Set(Up) (MultiNodeClusterSpec.scala:309)
[info] [JVM-1]   at org.scalatest.tools.Runner$.doRunRunRunDaDoRunRun(Runner.scala:1334)
[info] [JVM-4]   org.scalatest.exceptions.TestFailedException:
[info] [JVM-1]   at org.scalatest.tools.Runner$.$anonfun$runOptionallyWithPassFailReporter$24(Runner.scala:1031)
[info] [JVM-4]   at org.scalatest.MatchersHelper$.indicateFailure(MatchersHelper.scala:340)
[info] [JVM-1]   at org.scalatest.tools.Runner$.$anonfun$runOptionallyWithPassFailReporter$24$adapted(Runner.scala:1010)
...
[info] [JVM-3] - must reach initial convergence (on node 'third', class akka.cluster.UnreachableNodeJoinsAgainMultiJvmNode3) *** FAILED *** (25 seconds, 89 milliseconds)
[info] [JVM-3]   Set(Joining) did not equal Set(Up) (MultiNodeClusterSpec.scala:309)
[info] [JVM-3]   org.scalatest.exceptions.TestFailedException:
[info] [JVM-3]   at org.scalatest.MatchersHelper$.indicateFailure(MatchersHelper.scala:340)

@SethTisue
Copy link
Member

assuming this is also a 2.13.3 blocker unless/until I'm told otherwise

@retronym
Copy link
Member Author

b87dd51447233580aedbb340822e0c6dc0a41119 is the first bad commit
commit b87dd51447233580aedbb340822e0c6dc0a41119
/*
Author: Mike Skells <mike.skells@talk21.com>
Date:   Mon Feb 24 17:54:19 2020 +0000

    Backport 2.13 BedBlackTree to 2.12

      - Backports the new internal `RedBlackTree`.
      - In 2.12.x `Tree` must still extend `Serializable` as we don't
        use proxy based serialization for `TreeMap` / `TreeSet`.
      - Take advantage of the new implementation when the operand to
        `++`, `filter`, etc is the same collection type with the same
        `Ordering`. Many of these changes require us to modify the
        inherited implementation to add a type-case to be binary
        compatibile.

Next step is to try to drop the old TreeMap implementation selectively into akka-cluster (most to find where the behaviour changes). First guess is VectorClock.

@mkeskells
Copy link

Does this same issue occur in 2.13? The problematic commit was 99% just the 2.13 code

@retronym
Copy link
Member Author

A solution here is for the user to upgrade to Akk 2.5.20 or higher. This includes the changes to be compatible with bug fixes in the 2.13.0 collections: akka/akka#26043

for V in v2.12.11 2.12.x; do scala-ref $V -nc -e 'case class Foo(a: Int)(val b: Int) { override def toString = s"Foo($a)($b)" }; object Foo { implicit val o: Ordering[Foo] = Ordering.by(_.a) }; import collection.immutable._; println(Set(Foo(1)(1)).++(Set(Foo(1)(2))))'; done
Set(Foo(1)(1))
Set(Foo(1)(1))

 for V in v2.12.11 2.12.x; do scala-ref $V -nc -e 'case class Foo(a: Int)(val b: Int) { override def toString = s"Foo($a)($b)" }; object Foo { implicit val o: Ordering[Foo] = Ordering.by(_.b) }; import collection.immutable._; println(TreeSet(Foo(1)(1)).++(TreeSet(Foo(1)(2))))'; done
TreeSet(Foo(1)(1), Foo(1)(2))
TreeSet(Foo(1)(1), Foo(1)(2))

@retronym
Copy link
Member Author

I guess its still an open question whether we try to be bug-compatible in this area for 2.12.12.

@lrytz
Copy link
Member

lrytz commented Jun 16, 2020

So this is a question of which equal element is selected?

https://github.com/scala/scala/blob/db119f9be22674faf2cee3ced41050c07152a832/test/junit/scala/collection/SetMapRulesTest.scala#L24-L37

When updating a collection (immutably or in place), key element identities already in the source collection (i.e. the receiver of a method call) must always be preferred over new key elements which are equal but have different identities.

So the bug is in 2.13.2 (and in 2.12.x), or am I wrong?

2.12.11:

scala> import collection.immutable._
scala> case class Foo(a: Int)(override val toString: String); object Foo {
     |   implicit val o: Ordering[Foo] = Ordering.by(_.a)
     | }

scala> val x = Foo(1)("x")
scala> val y = Foo(1)("y")

scala> Set(x) ++ Set(y)
res0: scala.collection.immutable.Set[Foo] = Set(x)

scala> TreeSet(x) ++ Set(y)
res1: scala.collection.immutable.TreeSet[Foo] = TreeSet(x)

scala> TreeSet(x) ++ TreeSet(y)
res2: scala.collection.immutable.TreeSet[Foo] = TreeSet(x)

2.12.x and 2.13.2:

scala> Set(x) ++ Set(y)
res0: scala.collection.immutable.Set[Foo] = Set(x)

scala> TreeSet(x) ++ Set(y)
res1: scala.collection.immutable.TreeSet[Foo] = TreeSet(x)

scala> TreeSet(x) ++ TreeSet(y)
res2: scala.collection.immutable.TreeSet[Foo] = TreeSet(y)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants