Fix potential NoDelta leak into IReplicatedData.Merge() #4811
Conversation
| @@ -105,19 +104,17 @@ public void Delete(string key) | |||
| var cacheKey = (key, fromSeqNr, toSeqNr); | |||
| if (!cache.TryGetValue(cacheKey, out var deltaGroup)) | |||
| { | |||
| using (var e = deltaEntriesAfterJ.Values.GetEnumerator()) | |||
| deltaGroup = deltaEntriesAfterJ.Values.Aggregate((d1, d2) => | |||
There was a problem hiding this comment.
Replaced the suspicious looking code block with one that makes a lot more sense.
This is the scala code for reference:
val group = deltaEntriesAfterJ.valuesIterator.reduceLeft { (d1, d2) =>
val merged = d2 match {
case NoDeltaPlaceholder => NoDeltaPlaceholder
case _ =>
// this is fine also if d1 is a NoDeltaPlaceholder
d1.merge(d2.asInstanceOf[d1.T])
}
merged match {
case s: ReplicatedDeltaSize if s.deltaSize >= maxDeltaSize =>
// discard too large deltas
NoDeltaPlaceholder
case _ => merged
}
}reduceLeft works exactly the same as LINQ Aggregate
| deltaGroup = e.Current; | ||
| while (e.MoveNext()) | ||
| { | ||
| deltaGroup = deltaGroup.Merge(e.Current); |
There was a problem hiding this comment.
The bug happened here, a NoDelta null delta marker got passed into this line and ORDictionary could not cast it to DeltaGroup
| @@ -68,13 +69,11 @@ public void Delete(string key) | |||
| if (all.Length <= sliceSize) slice = all; | |||
| else | |||
| { | |||
| var start = (int)(_deltaNodeRoundRobinCounter % all.Length); | |||
There was a problem hiding this comment.
This part of the code also looks suspicious, so I re-ported it with a cleaner one.
Reference scala code:
val i = (deltaNodeRoundRobinCounter % all.size).toInt
val first = all.slice(i, i + sliceSize)
if (first.size == sliceSize) first
else first ++ all.take(sliceSize - first.size)
| } | ||
| slice = ImmutableArray.CreateRange(buffer); | ||
| var i = (int)(_deltaNodeRoundRobinCounter % all.Length); | ||
| slice = all.Slice(i, sliceSize).ToImmutableArray(); |
There was a problem hiding this comment.
This code looks different than the reference scala because in scala, slice parameters are from and to, while in C#, slice parameters are from and count
There was a problem hiding this comment.
Thanks - I was just about to ask
| } | ||
| slice = ImmutableArray.CreateRange(buffer); | ||
| var i = (int)(_deltaNodeRoundRobinCounter % all.Length); | ||
| slice = all.Slice(i, sliceSize).ToImmutableArray(); |
There was a problem hiding this comment.
Thanks - I was just about to ask
Related to #4806
Minimize the number of changes to only those that directly relates to the bug.