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

[SPARK-5569] [STREAMING] fix ObjectInputStreamWithLoader for supporting load array classes. #8955

Closed
wants to merge 3 commits into from

Conversation

@maxwellzdm
Copy link

commented Oct 1, 2015

When use Kafka DirectStream API to create checkpoint and restore saved checkpoint when restart,
ClassNotFound exception would occur.

The reason for this error is that ObjectInputStreamWithLoader extends the ObjectInputStream class and override its resolveClass method. But Instead of Using Class.forName(desc,false,loader), Spark uses loader.loadClass(desc) to instance the class, which do not works with array class.

For example:
Class.forName("[Lorg.apache.spark.streaming.kafka.OffsetRange.",false,loader) works well while loader.loadClass("[Lorg.apache.spark.streaming.kafka.OffsetRange") would throw an class not found exception.

details of the difference between Class.forName and loader.loadClass can be found here.
http://bugs.java.com/view_bug.do?bug_id=6446627

@tdas

This comment has been minimized.

Copy link
Contributor

commented Oct 16, 2015

ok to test

@tdas

This comment has been minimized.

Copy link
Contributor

commented Oct 16, 2015

I know it is hard but could you add unit tests. Some unit test that will fail without this patch.

@maxwellzdm

This comment has been minimized.

Copy link
Author

commented Oct 16, 2015

OK, Please give me some time and I'll try to add unit tests.

@tdas

This comment has been minimized.

Copy link
Contributor

commented Oct 16, 2015

Sure!

@tdas

This comment has been minimized.

Copy link
Contributor

commented Oct 16, 2015

this is ok to test

@SparkQA

This comment has been minimized.

Copy link

commented Oct 16, 2015

Test build #43854 has finished for PR 8955 at commit e21a12c.

  • This patch passes all tests.
  • This patch merges cleanly.
  • This patch adds no public classes.
@maxwellzdm

This comment has been minimized.

Copy link
Author

commented Oct 23, 2015

@tdas I have added unit test which wouldn't pass without this patch. Please review it when you have time.

@SparkQA

This comment has been minimized.

Copy link

commented Oct 23, 2015

Test build #44227 has finished for PR 8955 at commit d2d0404.

  • This patch passes all tests.
  • This patch merges cleanly.
  • This patch adds no public classes.
@tdas

This comment has been minimized.

Copy link
Contributor

commented Oct 27, 2015

LGTM. Merging this to master. Thank you very much!

@asfgit asfgit closed this in 17f4999 Oct 27, 2015

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
3 participants
You can’t perform that action at this time.