Uh oh!
There was an error while loading. Please reload this page.
[SPARK-2103][Streaming] Change to ClassTag for KafkaInputDStream and fix reflection issue - #1508
[SPARK-2103][Streaming] Change to ClassTag for KafkaInputDStream and fix reflection issue#1508jerryshao wants to merge 2 commits into
Conversation
SparkQA
commented
Jul 21, 2014
QA tests have started for PR 1508. This patch merges cleanly. |
srowen
commented
Jul 21, 2014
Nice one Jerry! this actually enables using Kafka with non-String data in Java. |
SparkQA
commented
Jul 21, 2014
QA results for PR 1508: |
srowen
commented
Jul 21, 2014
It's the MIMA test that fails, since the method signature is changed. It's possible to keep and deprecate the existing method of course. Should we just do that, or OK to remove the method on the grounds that the API doesn't quite work? |
jerryshao
commented
Jul 22, 2014
tdas
commented
Jul 30, 2014
Jenkins, test this please. |
SparkQA
commented
Jul 30, 2014
QA tests have started for PR 1508. This patch merges cleanly. |
tdas
commented
Jul 30, 2014
LGTM. I am okay with the binary compatibility change, since it is really wrong that we were using manifest instead of classtags. But I am not sure I really understand what is the reason behind the failure of the |
tdas
commented
Jul 30, 2014
Oh, please add the appropriate exclude in the Mima exclusions See the error in the jenkins console output for the exact exception string that you need to add. |
SparkQA
commented
Jul 30, 2014
QA results for PR 1508: |
tdas
commented
Jul 30, 2014
This failure is unrelated to this patch. Its a current known issue in Jenkins, will run the test again when this error gets cleared. |
jerryshao
commented
Jul 31, 2014
Hi TD, thanks for your review. I think the previous issue is that: implicitvalkeyCmd:Manifest[U] = implicitly[Manifest[AnyRef]].asInstanceOf[Manifest[U]]This line of code cannot actually get the correct runtime class, the runtime class of Because we only got valkeyDecoder= manifest[U].runtimeClass.getConstructor(classOf[VerifiableProperties])The right way is like this: implicitvalkeyCmd:Manifest[U] =Manifest.classType(keyDecodeClass)Manifest can also do the same thing if we correctly feed it with class info. But I think it would be better to change Manifest to ClassTag to keep align with other code, so I changed to use ClassTag. I will add this to Mima exclude. Thanks a lot. |
tdas
commented
Jul 31, 2014
I get it now. Let me try to run the Jenkins once again, to figure out what needs to be added to Mima |
tdas
commented
Jul 31, 2014
Jenkins, test this please. |
SparkQA
commented
Jul 31, 2014
QA tests have started for PR 1508. This patch merges cleanly. |
SparkQA
commented
Jul 31, 2014
QA results for PR 1508: |
tdas
commented
Jul 31, 2014
Again, this is an unrelated failure. Running it again. |
tdas
commented
Jul 31, 2014
Jenkins, test this please. |
SparkQA
commented
Jul 31, 2014
QA tests have started for PR 1508. This patch merges cleanly. |
SparkQA
commented
Jul 31, 2014
QA results for PR 1508: |
tdas
commented
Jul 31, 2014
Can you add these to the Mima excludes. The last one should not be necessary as it is not a public class. Let me look into this. No harm in adding the exclusion though. |
SparkQA
commented
Aug 1, 2014
QA tests have started for PR 1508. This patch DID NOT merge cleanly! |
tdas
commented
Aug 1, 2014
@jerryshao Oops, not sure how that is possible given what this patch touches. Can you merge with master nonetheless. |
jerryshao
commented
Aug 1, 2014
Ok, I will update the code. |
…ecoder construct issue when using Java API
SparkQA
commented
Aug 1, 2014
QA tests have started for PR 1508. This patch merges cleanly. |
SparkQA
commented
Aug 1, 2014
QA results for PR 1508: |
SparkQA
commented
Aug 1, 2014
QA results for PR 1508: |
tdas
commented
Aug 1, 2014
OK, this is very confusing, sequence of two results is very confusing. Let me run the tests again. |
tdas
commented
Aug 1, 2014
Jenkins, test this again. |
tdas
commented
Aug 1, 2014
Jenkins, test this please. |
SparkQA
commented
Aug 1, 2014
QA tests have started for PR 1508. This patch merges cleanly. |
srowen
commented
Aug 1, 2014
A bunch of tests have been failing spuriously with "java.net.BindException: Address already in use". It's not the PR. I wonder what recent change could have made this happen? Is something being stricter about assigning a fixed port? did a config change to let multiple tests run on the same virtual machine? |
tdas
commented
Aug 1, 2014
Also, a number of them have been failing for spurious python mllib issues. At least was the case yesterday. |
SparkQA
commented
Aug 1, 2014
QA results for PR 1508: |
tdas
commented
Aug 1, 2014
Heyyyaa, it finally passed. I am merging this. Thanks @jerryshao |
…fix reflection issue This PR updates previous Manifest for KafkaInputDStream's Decoder to ClassTag, also fix the problem addressed in [SPARK-2103](https://issues.apache.org/jira/browse/SPARK-2103). Previous Java interface cannot actually get the type of Decoder, so when using this Manifest to reconstruct the decode object will meet reflection exception. Also for other two Java interfaces, ClassTag[String] is useless because calling Scala API will get the right implicit ClassTag. Current Kafka unit test cannot actually verify the interface. I've tested these interfaces in my local and distribute settings. Author: jerryshao <saisai.shao@intel.com> Closesapache#1508 from jerryshao/SPARK-2103 and squashes the following commits: e90c37b [jerryshao] Add Mima excludes 7529810 [jerryshao] Change Manifest to ClassTag for KafkaInputDStream's Decoder and fix Decoder construct issue when using Java API
salex89
commented
Feb 16, 2015
I'm not sure if this is the exactly same issue, but I am experience this with Spark 1.2.1 and Scala 2.10.4 also. |
jerryshao
commented
Feb 20, 2015
Hi @salex89 , seems your exception is a little different to this PR, maybe I need a careful look at this exception, thanks for your reporting. |
salex89
commented
Feb 20, 2015
@jerryshao |
jerryshao
commented
Feb 21, 2015
Yeah, please do it and describe it in detail so we can easily reproduce it. |
…locks.enabled` by default (apache#1508) ### What changes were proposed in this pull request? rdar://99068608 (SPARK-40198 Enable `spark.storage.decommission.(rdd|shuffle)Blocks.enabled` by default) This PR aims to enable the following two configurations by default. Note that these storage configurations are disabled by default by the parent configuration, `spark.storage.decommission.enabled`, still. In addition, all decommission features including this storage decommissioning are disabled by `spark.decommission.enabled` by default. - `spark.storage.decommission.rddBlocks.enabled` - `spark.storage.decommission.shuffleBlocks.enabled` ### Why are the changes needed? This will help users use the storage decommissioning feature more easily. ### Does this PR introduce _any_ user-facing change? Yes, this will reduce the number of required configurations for decommission users. However, these two configurations are disabled by two configurations still. ### How was this patch tested? Pass the CIs.
This PR updates previous Manifest for KafkaInputDStream's Decoder to ClassTag, also fix the problem addressed in SPARK-2103.
Previous Java interface cannot actually get the type of Decoder, so when using this Manifest to reconstruct the decode object will meet reflection exception.
Also for other two Java interfaces, ClassTag[String] is useless because calling Scala API will get the right implicit ClassTag.
Current Kafka unit test cannot actually verify the interface. I've tested these interfaces in my local and distribute settings.