fix reflection bug: pass in different arguments for different version of same function #1037

marsishandsome · 2019-08-16T07:26:43Z

What problem does this PR solve?

this PR #1017 introduces a new bug:

Caused by: java.lang.IllegalArgumentException: wrong number of arguments
  at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
  at sun.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:62)
  at sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
  at java.lang.reflect.Method.invoke(Method.java:498)
  at com.pingcap.tispark.utils.ReflectionUtil$ReflectionMapPartitionWithIndexInternal.invoke(ReflectionUtil.scala:117)
  at org.apache.spark.sql.execution.CoprocessorRDD$$anonfun$doExecute$1.apply(CoprocessorRDD.scala:82)
  at org.apache.spark.sql.execution.CoprocessorRDD$$anonfun$doExecute$1.apply(CoprocessorRDD.scala:78)
  at scala.collection.immutable.List.map(List.scala:273)

How to fix

pass in different arguments for a different version of the same function

Cherry Pick

to master branch

marsishandsome · 2019-08-16T07:26:56Z

/run-all-tests

birdstorm

LGTM

birdstorm · 2019-08-16T11:09:12Z

a bit messy... perhaps we can ignore spark version and try these methods naively?

zhexuany · 2019-08-19T09:12:42Z

core/src/main/scala/com/pingcap/tispark/utils/ReflectionUtil.scala

+          try {
+            mapPartitionsWithIndexInternalV1(rdd, internalRowToUnsafeRowWithIndex)
+          } catch {
+            case _: Throwable =>


We catch all exception here. What if the exception thrown by spark is not what we want?

nice! fixed

marsishandsome · 2019-08-19T09:38:05Z

/run-all-tests

marsishandsome · 2019-08-19T09:38:29Z

/run-all-tests tikv=v3.0.1 tidb=v3.0.1 pd=v3.0.1

codecov-io · 2019-08-19T10:24:19Z

Codecov Report

❗ No coverage uploaded for pull request base (release-2.1@0585ece). Click here to learn what that means.
The diff coverage is n/a.

@@              Coverage Diff               @@
##             release-2.1    #1037   +/-   ##
==============================================
  Coverage               ?   53.57%           
  Complexity             ?     1128           
==============================================
  Files                  ?      153           
  Lines                  ?     7022           
  Branches               ?      816           
==============================================
  Hits                   ?     3762           
  Misses                 ?     2892           
  Partials               ?      368

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 0585ece...babf974. Read the comment docs.

marsishandsome · 2019-08-20T01:46:13Z

a bit messy... perhaps we can ignore spark version and try these methods naively?

Right, I can simply try these methods naively, like

try {
  use spark-2.3 function
} catch {
  use spark-2.4 function
}

When using spark-2.3, we will always succeed in running try.
But when using spark-2.4, we will always fail in running try, which we can avoid.

zhexuany

LGTM

birdstorm

LGTM

… of same function (pingcap#1037) (cherry picked from commit a5462c2)

… of same function (#1037) (#1052) (cherry picked from commit a5462c2)

* 2.2.0 -> 2.3.0 (#947) * Add tests for primary key (#948) * add changelog (#955) * add multi-column tests (#954) * fix range partition throw UnsupportedSyntaxException error (#960) * fix view parsing problem (#953) * make tispark can read from a hash partition table (#966) * increase ci worker number (#965) * update readme for tispark-2.1.2 release (#968) * update document for pyspark (#975) * fix one jar bug (#972) * adding common port number used by spark cluster (#973) * fix cost model in table scan (#977) * create an UninitializedType for TypeDecimal (#979) * update sparkr doc (#976) * use spark-2.4.3 to run ut (#978) * use spark-2.4.3 to run ut * fix ci * a better design for get auto table id (#980) * fix bug: ci SpecialTiDBTypeTestSuite failed with tidb-3.0.1 (#984) * improve TiConfiguration getPdAddrsString function (#963) * bump grpc to 1.17 (#982) * Add multiple-column PK tests (#970) * add retry for batchGet (#986) * use tispark self-made m2 cahce file (#990) * add spark sql document for batch write (#991) * add auto mode for test.data.load (#994) * fix typo (#996) * fix index scan bug (#995) * refine doc (#1003) * add tidb-3.0 compatibility document (#998) * add tidb-3.0 compatibility document * address code review * address code review * add log4j config document (#1008) * refactor batch write region pre-split (#999) * add ci simple mode (#1012) * clean up redundant code (#997) * prohibit agg or groupby pushdown on double read (#1004) * remove split region code (#1015) * add supported scala version (#1013) * Fix scala compiler version (#1010) * fix reflection bug for hdp release (#1017) (#1018) (cherry picked from commit 118b12e) * check by grammarly (#1022) * add benchmark result for batch write (#1025) * release tispark 2.1.3 (#1026) (#1035) (cherry picked from commit 107eb2b) * support setting random seed in daily regression test (#1032) * Remove create in tisession (#1021) * set tikv region size from 96M to 1M (#1031) * adding unique indices test for batch write (#1014) * use one unique seed (#1043) * remove unused code (#1030) * adding batch write pk insertion test (#1044) * fix table not found bug in TiSession because of synchronization (#1041) * fix test failure (#1051) * fix reflection bug: pass in different arguments for different version of same function (#1037) (#1052) (cherry picked from commit a5462c2) * Adding pk and unique index test for batch write (#1049) * fix distinct without alias bug: disable pushdown aggregate with alias (#1054) * improve the doc (#1053) * Refactor RegionStoreClient logic (#989) * using stream rather removeIf (#1057) * Remove redundant pre-write/commit logic in LockResolverTest (#1062) * adding recreate flag when create tisession (#1064) * fix issue 1047 (#1066) * cleanup code in TiBatchWrite (#1067) * release tispark-2.1.4 (#1068) (#1069) (cherry picked from commit fd8068a) * update document for tispark-2.1.4 release (#1070)

… of same function (pingcap#1037) (pingcap#1052) (cherry picked from commit a5462c2)

fix reflection bug

b301aa9

marsishandsome requested a review from birdstorm August 16, 2019 07:51

birdstorm previously approved these changes Aug 16, 2019

View reviewed changes

marsishandsome changed the title ~~fix reflection bug~~ [DNM] fix reflection bug Aug 19, 2019

marsishandsome changed the title ~~[DNM] fix reflection bug~~ fix reflection bug: pass in different arguments Aug 19, 2019

marsishandsome requested review from birdstorm and zhexuany August 19, 2019 07:46

marsishandsome changed the title ~~fix reflection bug: pass in different arguments~~ fix reflection bug: pass in different arguments for different version of same function Aug 19, 2019

zhexuany reviewed Aug 19, 2019

View reviewed changes

address code review

babf974

marsishandsome dismissed birdstorm’s stale review via babf974 August 19, 2019 09:36

zhexuany approved these changes Aug 20, 2019

View reviewed changes

birdstorm approved these changes Aug 20, 2019

View reviewed changes

marsishandsome merged commit a5462c2 into pingcap:release-2.1 Aug 20, 2019

marsishandsome added a commit to marsishandsome/tispark that referenced this pull request Aug 20, 2019

fix reflection bug: pass in different arguments for different version…

de4fb7d

… of same function (pingcap#1037) (cherry picked from commit a5462c2)

marsishandsome mentioned this pull request Aug 20, 2019

[cherry-pick] fix reflection bug: pass in different arguments for different version… #1052

Merged

marsishandsome added a commit to marsishandsome/tispark that referenced this pull request Aug 21, 2019

fix reflection bug: pass in different arguments for different version…

1b43f87

… of same function (pingcap#1037) (cherry picked from commit a5462c2)

marsishandsome added a commit that referenced this pull request Aug 21, 2019

fix reflection bug: pass in different arguments for different version…

bb7c646

… of same function (#1037) (#1052) (cherry picked from commit a5462c2)

wfxxh pushed a commit to wanfangdata/tispark that referenced this pull request Jun 30, 2023

fix reflection bug: pass in different arguments for different version…

8b6f1a3

… of same function (pingcap#1037) (pingcap#1052) (cherry picked from commit a5462c2)

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix reflection bug: pass in different arguments for different version of same function #1037

fix reflection bug: pass in different arguments for different version of same function #1037

marsishandsome commented Aug 16, 2019 •

edited

Loading

marsishandsome commented Aug 16, 2019

birdstorm left a comment

birdstorm commented Aug 16, 2019

zhexuany Aug 19, 2019

marsishandsome Aug 19, 2019

marsishandsome commented Aug 19, 2019

marsishandsome commented Aug 19, 2019

codecov-io commented Aug 19, 2019

marsishandsome commented Aug 20, 2019

zhexuany left a comment

birdstorm left a comment

fix reflection bug: pass in different arguments for different version of same function #1037

fix reflection bug: pass in different arguments for different version of same function #1037

Conversation

marsishandsome commented Aug 16, 2019 • edited Loading

What problem does this PR solve?

How to fix

Cherry Pick

marsishandsome commented Aug 16, 2019

birdstorm left a comment

Choose a reason for hiding this comment

birdstorm commented Aug 16, 2019

zhexuany Aug 19, 2019

Choose a reason for hiding this comment

marsishandsome Aug 19, 2019

Choose a reason for hiding this comment

marsishandsome commented Aug 19, 2019

marsishandsome commented Aug 19, 2019

codecov-io commented Aug 19, 2019

Codecov Report

marsishandsome commented Aug 20, 2019

zhexuany left a comment

Choose a reason for hiding this comment

birdstorm left a comment

Choose a reason for hiding this comment

marsishandsome commented Aug 16, 2019 •

edited

Loading