-
Notifications
You must be signed in to change notification settings - Fork 175
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
Create sequential serializer failed #1325
Comments
Hi @andyczerwonka , thanks for reporting this bug. Can this bug be reproduced locally? |
No, we’re seeing it intermittently. |
@andyczerwonka Coud you use lastest snapshot jar? Dose it work for you? |
@andyczerwonka I try to fix it in #1333, but I'm not sure whether they are same issue. Could you try this branch in your environment? |
Which branch? |
Do it address your issue ? @andyczerwonka |
I haven’t been able to test it yet, but I will be doing it either today or
tomorrow morning.
…On Fri, Jan 12, 2024 at 2:40 AM Shawn Yang ***@***.***> wrote:
Do it work for you ? @andyczerwonka <https://github.com/andyczerwonka>
—
Reply to this email directly, view it on GitHub
<#1325 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AABB5ETZ2ZP25FCUPAB2RO3YOEAKNAVCNFSM6AAAAABBTPJ37KVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMYTQOBYG42TIOJXHA>
.
You are receiving this because you were mentioned.Message ID:
***@***.***>
|
Fix nested collection cast for scala, we missed nested scala collection/map check before Closes #1325
I’m still working on reproduction steps. |
Hi @andyczerwonka, has this issue been addressed? |
@chaokunyang I have been able to reproduce. I now need instructions as to how to build the jar with your fix to validate the fix. Or you can attach it here if that's easier. |
@andyczerwonka You can just use <repositories>
<repository>
<id>apache</id>
<url>https://repository.apache.org/snapshots/</url>
<releases>
<enabled>false</enabled>
</releases>
<snapshots>
<enabled>true</enabled>
</snapshots>
</repository>
</repositories>
<dependency>
<groupId>org.apache.fury</groupId>
<artifactId>fury-core</artifactId>
<version>0.5.0-SNAPSHOT</version>
</dependency>
<!-- row/arrow format support -->
<!-- <dependency>
<groupId>org.apache.fury</groupId>
<artifactId>fury-format</artifactId>
<version>0.5.0-SNAPSHOT</version>
</dependency> -->
|
Can your reproduction code be converted into a unit test? Would be great if we can include it into fury unit test |
I have tested it with the
|
It's late here, so first thing tomorrow I will try to share the code that reproduces the error. |
I have created an example of the failure. |
Hi @andyczerwonka , I tested with your code, Fury succeeds in serialization: package org.apache.fury.serializer;
import org.apache.fury.Fury
import org.apache.fury.config.Language
import java.io.ByteArrayInputStream
import java.io.ByteArrayOutputStream
import java.util.Base64
import java.util.zip.GZIPInputStream
import java.util.zip.GZIPOutputStream
import _root_.scala
import _root_.scala.util.{Success, Try}
import _root_.scala.concurrent.Future
import _root_.scala.concurrent.ExecutionContext
import java.util.concurrent.ThreadLocalRandom
import _root_.scala.concurrent.Await
import org.apache.fury.ThreadLocalFury
import org.scalatest.matchers.should.Matchers
import org.scalatest.wordspec.AnyWordSpec
// What makes this fail is the nested collection in the case class. If you change it to
// as 1-dimensional collection, we no longer see the exception
case class SampleData(label: String, data: Seq[Seq[Int]])
class SerdeThreadingTest extends AnyWordSpec with Matchers {
def threadLocalFury =
new ThreadLocalFury(classLoader => {
Fury
.builder()
.withLanguage(Language.JAVA)
.requireClassRegistration(false)
.withScalaOptimizationEnabled(true)
.withRefTracking(true)
.withStringCompressed(true)
.withLongCompressed(true)
.withIntCompressed(true)
.withAsyncCompilation(true)
.withClassLoader(classLoader)
.build()
})
private val fury = Fury
.builder()
.withLanguage(Language.JAVA)
.requireClassRegistration(false)
.withScalaOptimizationEnabled(true)
.withRefTracking(true)
.withStringCompressed(true)
.withLongCompressed(true)
.withIntCompressed(true)
.withAsyncCompilation(true)
.buildThreadSafeFury()
def encode(sampleData: SampleData) = {
val raw = fury.serialize(sampleData)
val bos = new ByteArrayOutputStream(raw.length)
val zos = new GZIPOutputStream(bos)
zos.write(raw)
zos.flush()
zos.close()
bos.close()
sleepBetween(500, 1000)
Base64.getEncoder.encodeToString(bos.toByteArray)
}
def decode(encoded: String) =
Try {
val bis = new ByteArrayInputStream(Base64.getDecoder.decode(encoded))
val zis = new GZIPInputStream(bis)
val uncompressed = zis.readAllBytes()
val result = fury.deserialize(uncompressed).asInstanceOf[SampleData]
zis.close()
bis.close()
sleepBetween(500, 1000)
result
}
def sleepBetween(min: Int, max: Int) = {
val sleepTime = ThreadLocalRandom.current().nextInt(min, max)
Thread.sleep(sleepTime.toLong)
}
"fury scala object support" should {
"testNonThreadedSerde" in {
val data = SampleData("single sample", Seq.empty)
val encoded = encode(data)
val decoded = decode(encoded)
println(decoded)
assert(decoded == Success(data))
}
"testNestedCollectionThreadedSerde" in {
import scala.concurrent.duration._
implicit val ec = ExecutionContext.global
val tasks = for (i <- 1 to 1) yield Future {
val data = SampleData(i.toString, Seq.empty)
val encoded = encode(data)
encoded
}
val decodedFuture = for {
f <- Future.sequence(tasks)
} yield for {
encoded <- f
} yield {
val Success(decoded) = decode(encoded)
decoded
}
val result = Await.result(decodedFuture, 20.seconds)
println(result)
assert(result.size == 1)
}
}
}
|
I guess you didn't use latest snapshot version of Fury. The package name has been changed from |
Is the nested collection issue fixed in the latest snapshot? Also, can you change the code to run 10-20 threads? for (i <- 1 to 10) yield Future
|
Which is probably why it didn’t get pulled in. When can we expect a non-snapshot release? |
You need to add apache snapshot repo: <repositories>
<repository>
<id>apache</id>
<url>https://repository.apache.org/snapshots/</url>
<releases>
<enabled>false</enabled>
</releases>
<snapshots>
<enabled>true</enabled>
</snapshots>
</repository>
</repositories>
<dependency>
<groupId>org.apache.fury</groupId>
<artifactId>fury-core</artifactId>
<version>0.5.0-SNAPSHOT</version>
</dependency> The non-snapshot release may need a month before we release it. |
@chaokunyang I have confirmed that the issue has been resolved in 0.5.0. Is there a work-around for this issue? Can it be back-ported to something that can be released earlier? We are in a non-working state and can't move forward. |
Could you build a jar and use that jar? We may need some time before we release a new jar. We have some issue like license needs being addressed and release blog before we can release a new version |
Defect is addressed in the latest snapshot. |
## What does this PR do? Some collectionSerializer may overwrite write/read method, then clear element serializer may not got invoked. This PR clears serializer for collection/map to avoid container use wrong serializer for nested elements. ## Related issues #1558 #1455, #1325 and #1176. ## Does this PR introduce any user-facing change? <!-- If any user-facing interface changes, please [open an issue](https://github.com/apache/incubator-fury/issues/new/choose) describing the need to do so and update the document if necessary. --> - [ ] Does this PR introduce any public API change? - [ ] Does this PR introduce any binary protocol compatibility change? ## Benchmark <!-- When the PR has an impact on performance (if you don't know whether the PR will have an impact on performance, you can submit the PR first, and if it will have impact on performance, the code reviewer will explain it), be sure to attach a benchmark data here. -->
Search before asking
It may be a duplicate of #1176, but the trace suggests it might be a different cause.
Version
0.4.1
Component(s)
Java
Minimal reproduce step
Run the serializer against our object graph. It seems to happen intermittently, so could be a threading issue.
What did you expect to see?
No runtime exception.
What did you see instead?
Anything Else?
I might be doing something wrong, but would like to help to triage.
Are you willing to submit a PR?
The text was updated successfully, but these errors were encountered: