You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@spark.apache.org by GitBox <gi...@apache.org> on 2020/04/12 14:51:01 UTC
[GitHub] [spark] Ngone51 opened a new pull request #28195:
[SPARK-31425][SQL][CORE] UnsafeKVExternalSorter should also respect
UnsafeAlignedOffset
Ngone51 opened a new pull request #28195: [SPARK-31425][SQL][CORE] UnsafeKVExternalSorter should also respect UnsafeAlignedOffset
URL: https://github.com/apache/spark/pull/28195
<!--
Thanks for sending a pull request! Here are some tips for you:
1. If this is your first time, please read our contributor guidelines: https://spark.apache.org/contributing.html
2. Ensure you have added or run the appropriate tests for your PR: https://spark.apache.org/developer-tools.html
3. If the PR is unfinished, add '[WIP]' in your PR title, e.g., '[WIP][SPARK-XXXX] Your PR title ...'.
4. Be sure to keep the PR description updated to reflect all changes.
5. Please write your PR title to summarize what this PR proposes.
6. If possible, provide a concise example to reproduce the issue for a faster review.
7. If you want to add a new configuration, please read the guideline first for naming configurations in
'core/src/main/scala/org/apache/spark/internal/config/ConfigEntry.scala'.
-->
### What changes were proposed in this pull request?
<!--
Please clarify what changes you are proposing. The purpose of this section is to outline the changes and how this PR fixes the issue.
If possible, please consider writing useful notes for better and faster reviews in your PR. See the examples below.
1. If you refactor some codes with changing classes, showing the class hierarchy will help reviewers.
2. If you fix some SQL features, you can provide some references of other DBMSes.
3. If there is design documentation, please add the link.
4. If there is a discussion in the mailing list, please add the link.
-->
Make `UnsafeKVExternalSorter` also respect `UnsafeAlignedOffset` when reading the record and update some out of date comemnts.
### Why are the changes needed?
<!--
Please clarify why the changes are needed. For instance,
1. If you propose a new API, clarify the use case for a new API.
2. If you fix a bug, you can clarify why it is a bug.
-->
Since `BytesToBytesMap` respects `UnsafeAlignedOffset` when writing the record, `UnsafeKVExternalSorter` should also respect `UnsafeAlignedOffset` when reading the record otherwise it will causes data correctness issue.
### Does this PR introduce any user-facing change?
<!--
If yes, please clarify the previous behavior and the change this PR proposes - provide the console output, description and/or an example to show the behavior difference if possible.
If no, write 'No'.
-->
No.
### How was this patch tested?
<!--
If tests were added, say they were added here. Please make sure to add some test cases that check the changes thoroughly including negative and positive cases if possible.
If it was tested in a way different from regular unit tests, please clarify how you tested step by step, ideally copy and paste-able, so that other reviewers can test and check, and descendants can verify in the future.
If tests were not added, please describe why they were not added and/or why it was difficult to add.
-->
Manually tested `HashAggregationQueryWithControlledFallbackSuite` with `UAO_SIZE=8` to simulate SPARC platform. And tests only pass with this fix.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins commented on issue #28195:
[SPARK-31425][SQL][CORE]
UnsafeKVExternalSorter/VariableLengthRowBasedKeyValueBatch should also
respect UnsafeAlignedOffset
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on issue #28195: [SPARK-31425][SQL][CORE] UnsafeKVExternalSorter/VariableLengthRowBasedKeyValueBatch should also respect UnsafeAlignedOffset
URL: https://github.com/apache/spark/pull/28195#issuecomment-614119835
Merged build finished. Test PASSed.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins removed a comment on issue #28195:
[SPARK-31425][SQL][CORE]
UnsafeKVExternalSorter/VariableLengthRowBasedKeyValueBatch should also
respect UnsafeAlignedOffset
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on issue #28195: [SPARK-31425][SQL][CORE] UnsafeKVExternalSorter/VariableLengthRowBasedKeyValueBatch should also respect UnsafeAlignedOffset
URL: https://github.com/apache/spark/pull/28195#issuecomment-613772069
Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder-K8s/25981/
Test PASSed.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] cloud-fan commented on a change in pull request #28195:
[SPARK-31425][SQL][CORE]
UnsafeKVExternalSorter/VariableLengthRowBasedKeyValueBatch should also
respect UnsafeAlignedOffset
Posted by GitBox <gi...@apache.org>.
cloud-fan commented on a change in pull request #28195: [SPARK-31425][SQL][CORE] UnsafeKVExternalSorter/VariableLengthRowBasedKeyValueBatch should also respect UnsafeAlignedOffset
URL: https://github.com/apache/spark/pull/28195#discussion_r407951191
##########
File path: sql/catalyst/src/main/java/org/apache/spark/sql/catalyst/expressions/VariableLengthRowBasedKeyValueBatch.java
##########
@@ -141,14 +145,16 @@ public boolean next() {
return false;
}
- totalLength = Platform.getInt(base, offsetInPage) - 4;
- currentklen = Platform.getInt(base, offsetInPage + 4);
+ int uaoSize = UnsafeAlignedOffset.getUaoSize();
+ int doubleUaoSize = uaoSize + uaoSize;
Review comment:
let's be consistent with https://github.com/apache/spark/pull/28195/files#diff-ed82c6d7b012a3bf89fc2974e2840deaR46
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] Ngone51 commented on a change in pull request #28195:
[SPARK-31425][SQL][CORE] UnsafeKVExternalSorter should also respect
UnsafeAlignedOffset
Posted by GitBox <gi...@apache.org>.
Ngone51 commented on a change in pull request #28195: [SPARK-31425][SQL][CORE] UnsafeKVExternalSorter should also respect UnsafeAlignedOffset
URL: https://github.com/apache/spark/pull/28195#discussion_r407337486
##########
File path: core/src/main/java/org/apache/spark/unsafe/map/BytesToBytesMap.java
##########
@@ -706,7 +706,7 @@ public boolean append(Object kbase, long koff, int klen, Object vbase, long voff
// Here, we'll copy the data into our data pages. Because we only store a relative offset from
// the key address instead of storing the absolute address of the value, the key and value
// must be stored in the same memory page.
- // (8 byte key length) (key) (value) (8 byte pointer to next value)
+ // (uao: klen + vlen + uaolen) (uao: klen) (key) (value) (8 byte pointer to next value)
Review comment:
yes, if `total length` here doesn't include the first `uaoSize`.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] kiszk commented on a change in pull request #28195:
[SPARK-31425][SQL][CORE]
UnsafeKVExternalSorter/VariableLengthRowBasedKeyValueBatch should also
respect UnsafeAlignedOffset
Posted by GitBox <gi...@apache.org>.
kiszk commented on a change in pull request #28195: [SPARK-31425][SQL][CORE] UnsafeKVExternalSorter/VariableLengthRowBasedKeyValueBatch should also respect UnsafeAlignedOffset
URL: https://github.com/apache/spark/pull/28195#discussion_r408356089
##########
File path: sql/hive/src/test/scala/org/apache/spark/sql/hive/execution/AggregationQuerySuite.scala
##########
@@ -1055,28 +1056,31 @@ class HashAggregationQueryWithControlledFallbackSuite extends AggregationQuerySu
Seq("true", "false").foreach { enableTwoLevelMaps =>
withSQLConf(SQLConf.ENABLE_TWOLEVEL_AGG_MAP.key ->
enableTwoLevelMaps) {
- (1 to 3).foreach { fallbackStartsAt =>
- withSQLConf("spark.sql.TungstenAggregate.testFallbackStartsAt" ->
- s"${(fallbackStartsAt - 1).toString}, ${fallbackStartsAt.toString}") {
- // Create a new df to make sure its physical operator picks up
- // spark.sql.TungstenAggregate.testFallbackStartsAt.
- // todo: remove it?
- val newActual = Dataset.ofRows(spark, actual.logicalPlan)
-
- QueryTest.getErrorMessageInCheckAnswer(newActual, expectedAnswer) match {
- case Some(errorMessage) =>
- val newErrorMessage =
- s"""
- |The following aggregation query failed when using HashAggregate with
- |controlled fallback (it falls back to bytes to bytes map once it has processed
- |${fallbackStartsAt - 1} input rows and to sort-based aggregation once it has
- |processed $fallbackStartsAt input rows). The query is ${actual.queryExecution}
- |
- |$errorMessage
- """.stripMargin
-
- fail(newErrorMessage)
- case None => // Success
+ Seq(4, 8).foreach { uaoSize =>
+ UnsafeAlignedOffset.setUaoSize(uaoSize)
+ (1 to 3).foreach { fallbackStartsAt =>
+ withSQLConf("spark.sql.TungstenAggregate.testFallbackStartsAt" ->
+ s"${(fallbackStartsAt - 1).toString}, ${fallbackStartsAt.toString}") {
+ // Create a new df to make sure its physical operator picks up
+ // spark.sql.TungstenAggregate.testFallbackStartsAt.
+ // todo: remove it?
+ val newActual = Dataset.ofRows(spark, actual.logicalPlan)
+
+ QueryTest.getErrorMessageInCheckAnswer(newActual, expectedAnswer) match {
+ case Some(errorMessage) =>
+ val newErrorMessage =
+ s"""
+ |The following aggregation query failed when using HashAggregate with
+ |controlled fallback (it falls back to bytes to bytes map once it has
+ |processed ${fallbackStartsAt - 1} input rows and to sort-based aggregation
+ |once it has processed $fallbackStartsAt input rows).
+ |The query is ${actual.queryExecution}
+ |$errorMessage
+ """.stripMargin
+
+ fail(newErrorMessage)
+ case None => // Success
+ }
Review comment:
I am not sure, but do we need to call `setUaoSize(0)` at the end of this test since `TEST_UAO_SIZE` is static.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins commented on issue #28195:
[SPARK-31425][SQL][CORE]
UnsafeKVExternalSorter/VariableLengthRowBasedKeyValueBatch should also
respect UnsafeAlignedOffset
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on issue #28195: [SPARK-31425][SQL][CORE] UnsafeKVExternalSorter/VariableLengthRowBasedKeyValueBatch should also respect UnsafeAlignedOffset
URL: https://github.com/apache/spark/pull/28195#issuecomment-613532610
Merged build finished. Test FAILed.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] Ngone51 commented on issue #28195: [SPARK-31425][SQL][CORE]
UnsafeKVExternalSorter/VariableLengthRowBasedKeyValueBatch should also
respect UnsafeAlignedOffset
Posted by GitBox <gi...@apache.org>.
Ngone51 commented on issue #28195: [SPARK-31425][SQL][CORE] UnsafeKVExternalSorter/VariableLengthRowBasedKeyValueBatch should also respect UnsafeAlignedOffset
URL: https://github.com/apache/spark/pull/28195#issuecomment-615081591
thanks all!
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] Ngone51 commented on a change in pull request #28195:
[SPARK-31425][SQL][CORE]
UnsafeKVExternalSorter/VariableLengthRowBasedKeyValueBatch should also
respect UnsafeAlignedOffset
Posted by GitBox <gi...@apache.org>.
Ngone51 commented on a change in pull request #28195: [SPARK-31425][SQL][CORE] UnsafeKVExternalSorter/VariableLengthRowBasedKeyValueBatch should also respect UnsafeAlignedOffset
URL: https://github.com/apache/spark/pull/28195#discussion_r408609783
##########
File path: common/unsafe/src/main/java/org/apache/spark/unsafe/UnsafeAlignedOffset.java
##########
@@ -28,12 +28,20 @@
private static final int UAO_SIZE = Platform.unaligned() ? 4 : 8;
+ private static int TEST_UAO_SIZE = 0;
+
+ // used for test only
+ public static void setUaoSize(int size) {
+ assert size == 0 || size == 4 || size == 8;
+ TEST_UAO_SIZE = size;
+ }
+
public static int getUaoSize() {
- return UAO_SIZE;
+ return TEST_UAO_SIZE == 0 ? UAO_SIZE : TEST_UAO_SIZE;
Review comment:
Other tests might not set UAO size manually, then we'll get 0 in this case.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] kiszk commented on issue #28195: [SPARK-31425][SQL][CORE]
UnsafeKVExternalSorter/VariableLengthRowBasedKeyValueBatch should also
respect UnsafeAlignedOffset
Posted by GitBox <gi...@apache.org>.
kiszk commented on issue #28195: [SPARK-31425][SQL][CORE] UnsafeKVExternalSorter/VariableLengthRowBasedKeyValueBatch should also respect UnsafeAlignedOffset
URL: https://github.com/apache/spark/pull/28195#issuecomment-613592062
Thank you for updating `VariableLengthRowBasedKeyValueBatch` and your clarification. Looks good.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins removed a comment on issue #28195:
[SPARK-31425][SQL][CORE]
UnsafeKVExternalSorter/VariableLengthRowBasedKeyValueBatch should also
respect UnsafeAlignedOffset
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on issue #28195: [SPARK-31425][SQL][CORE] UnsafeKVExternalSorter/VariableLengthRowBasedKeyValueBatch should also respect UnsafeAlignedOffset
URL: https://github.com/apache/spark/pull/28195#issuecomment-613532619
Test FAILed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/121276/
Test FAILed.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins removed a comment on issue #28195:
[SPARK-31425][SQL][CORE]
UnsafeKVExternalSorter/VariableLengthRowBasedKeyValueBatch should also
respect UnsafeAlignedOffset
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on issue #28195: [SPARK-31425][SQL][CORE] UnsafeKVExternalSorter/VariableLengthRowBasedKeyValueBatch should also respect UnsafeAlignedOffset
URL: https://github.com/apache/spark/pull/28195#issuecomment-613368576
Merged build finished. Test FAILed.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA commented on issue #28195: [SPARK-31425][SQL][CORE]
UnsafeKVExternalSorter/VariableLengthRowBasedKeyValueBatch should also
respect UnsafeAlignedOffset
Posted by GitBox <gi...@apache.org>.
SparkQA commented on issue #28195: [SPARK-31425][SQL][CORE] UnsafeKVExternalSorter/VariableLengthRowBasedKeyValueBatch should also respect UnsafeAlignedOffset
URL: https://github.com/apache/spark/pull/28195#issuecomment-613643037
**[Test build #121284 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/121284/testReport)** for PR 28195 at commit [`bda5823`](https://github.com/apache/spark/commit/bda58233051227a679042abf77f859effbe29169).
* This patch passes all tests.
* This patch merges cleanly.
* This patch adds no public classes.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins removed a comment on issue #28195:
[SPARK-31425][SQL][CORE] UnsafeKVExternalSorter should also respect
UnsafeAlignedOffset
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on issue #28195: [SPARK-31425][SQL][CORE] UnsafeKVExternalSorter should also respect UnsafeAlignedOffset
URL: https://github.com/apache/spark/pull/28195#issuecomment-612628250
Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder-K8s/25834/
Test PASSed.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA commented on issue #28195: [SPARK-31425][SQL][CORE]
UnsafeKVExternalSorter/VariableLengthRowBasedKeyValueBatch should also
respect UnsafeAlignedOffset
Posted by GitBox <gi...@apache.org>.
SparkQA commented on issue #28195: [SPARK-31425][SQL][CORE] UnsafeKVExternalSorter/VariableLengthRowBasedKeyValueBatch should also respect UnsafeAlignedOffset
URL: https://github.com/apache/spark/pull/28195#issuecomment-614788138
**[Test build #121368 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/121368/testReport)** for PR 28195 at commit [`c6f39b5`](https://github.com/apache/spark/commit/c6f39b5a25e1e92c7f5a71ca541e9ce501850000).
* This patch passes all tests.
* This patch merges cleanly.
* This patch adds no public classes.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] cloud-fan commented on a change in pull request #28195:
[SPARK-31425][SQL][CORE] UnsafeKVExternalSorter should also respect
UnsafeAlignedOffset
Posted by GitBox <gi...@apache.org>.
cloud-fan commented on a change in pull request #28195: [SPARK-31425][SQL][CORE] UnsafeKVExternalSorter should also respect UnsafeAlignedOffset
URL: https://github.com/apache/spark/pull/28195#discussion_r407322984
##########
File path: core/src/main/java/org/apache/spark/unsafe/map/BytesToBytesMap.java
##########
@@ -706,7 +706,7 @@ public boolean append(Object kbase, long koff, int klen, Object vbase, long voff
// Here, we'll copy the data into our data pages. Because we only store a relative offset from
// the key address instead of storing the absolute address of the value, the key and value
// must be stored in the same memory page.
- // (8 byte key length) (key) (value) (8 byte pointer to next value)
+ // (uao: klen + vlen + uaolen) (uao: klen) (key) (value) (8 byte pointer to next value)
Review comment:
what does `(uao: klen + vlen + uaolen) (uao: klen)` mean?
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins removed a comment on issue #28195:
[SPARK-31425][SQL][CORE]
UnsafeKVExternalSorter/VariableLengthRowBasedKeyValueBatch should also
respect UnsafeAlignedOffset
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on issue #28195: [SPARK-31425][SQL][CORE] UnsafeKVExternalSorter/VariableLengthRowBasedKeyValueBatch should also respect UnsafeAlignedOffset
URL: https://github.com/apache/spark/pull/28195#issuecomment-614015397
Merged build finished. Test PASSed.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] Ngone51 commented on issue #28195: [SPARK-31425][SQL][CORE]
UnsafeKVExternalSorter should also respect UnsafeAlignedOffset
Posted by GitBox <gi...@apache.org>.
Ngone51 commented on issue #28195: [SPARK-31425][SQL][CORE] UnsafeKVExternalSorter should also respect UnsafeAlignedOffset
URL: https://github.com/apache/spark/pull/28195#issuecomment-613280460
I fixed `VariableLengthRowBasedKeyValueBatch` as well. To be clarify, it won't cause data correctness issue as `UnsafeKVExternalSorter` does. Unlike `UnsafeKVExternalSorter` may reading records from `BytesToBytesMap`, `VariableLengthRowBasedKeyValueBatch` writes and reads records by itself. Thus, similar to #22053 and [your comment](https://github.com/apache/spark/pull/22053#issuecomment-411975239) there, it's more likely an improvement for the support of SPARC platform.
BTW, I believe we don't need to fix `FixedLengthRowBasedKeyValueBatch` as it doesn't need to write any length. cc @kiszk
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA commented on issue #28195: [SPARK-31425][SQL][CORE]
UnsafeKVExternalSorter/VariableLengthRowBasedKeyValueBatch should also
respect UnsafeAlignedOffset
Posted by GitBox <gi...@apache.org>.
SparkQA commented on issue #28195: [SPARK-31425][SQL][CORE] UnsafeKVExternalSorter/VariableLengthRowBasedKeyValueBatch should also respect UnsafeAlignedOffset
URL: https://github.com/apache/spark/pull/28195#issuecomment-613818040
**[Test build #121297 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/121297/testReport)** for PR 28195 at commit [`d441e53`](https://github.com/apache/spark/commit/d441e5375859d2339334e7fd431b3c635407c516).
* This patch passes all tests.
* This patch merges cleanly.
* This patch adds no public classes.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA commented on issue #28195: [SPARK-31425][SQL][CORE]
UnsafeKVExternalSorter/VariableLengthRowBasedKeyValueBatch should also
respect UnsafeAlignedOffset
Posted by GitBox <gi...@apache.org>.
SparkQA commented on issue #28195: [SPARK-31425][SQL][CORE] UnsafeKVExternalSorter/VariableLengthRowBasedKeyValueBatch should also respect UnsafeAlignedOffset
URL: https://github.com/apache/spark/pull/28195#issuecomment-613531437
**[Test build #121276 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/121276/testReport)** for PR 28195 at commit [`bda5823`](https://github.com/apache/spark/commit/bda58233051227a679042abf77f859effbe29169).
* This patch **fails SparkR unit tests**.
* This patch merges cleanly.
* This patch adds no public classes.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] Ngone51 commented on a change in pull request #28195:
[SPARK-31425][SQL][CORE]
UnsafeKVExternalSorter/VariableLengthRowBasedKeyValueBatch should also
respect UnsafeAlignedOffset
Posted by GitBox <gi...@apache.org>.
Ngone51 commented on a change in pull request #28195: [SPARK-31425][SQL][CORE] UnsafeKVExternalSorter/VariableLengthRowBasedKeyValueBatch should also respect UnsafeAlignedOffset
URL: https://github.com/apache/spark/pull/28195#discussion_r408615862
##########
File path: common/unsafe/src/main/java/org/apache/spark/unsafe/UnsafeAlignedOffset.java
##########
@@ -28,12 +28,20 @@
private static final int UAO_SIZE = Platform.unaligned() ? 4 : 8;
+ private static int TEST_UAO_SIZE = 0;
+
+ // used for test only
+ public static void setUaoSize(int size) {
+ assert size == 0 || size == 4 || size == 8;
+ TEST_UAO_SIZE = size;
+ }
+
public static int getUaoSize() {
- return UAO_SIZE;
+ return TEST_UAO_SIZE == 0 ? UAO_SIZE : TEST_UAO_SIZE;
Review comment:
Ok, I get your point. Let me update it later.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] kiszk commented on issue #28195: [SPARK-31425][SQL][CORE]
UnsafeKVExternalSorter should also respect UnsafeAlignedOffset
Posted by GitBox <gi...@apache.org>.
kiszk commented on issue #28195: [SPARK-31425][SQL][CORE] UnsafeKVExternalSorter should also respect UnsafeAlignedOffset
URL: https://github.com/apache/spark/pull/28195#issuecomment-612826485
@Ngone51 Did you run other tests in the environment at uao=8.
I am curious that the following files also depends on `uao=4`.
https://github.com/apache/spark/blob/master/sql/catalyst/src/main/java/org/apache/spark/sql/catalyst/expressions/VariableLengthRowBasedKeyValueBatch.java
https://github.com/apache/spark/blob/master/sql/catalyst/src/main/java/org/apache/spark/sql/catalyst/expressions/FixedLengthRowBasedKeyValueBatch.java
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA removed a comment on issue #28195:
[SPARK-31425][SQL][CORE]
UnsafeKVExternalSorter/VariableLengthRowBasedKeyValueBatch should also
respect UnsafeAlignedOffset
Posted by GitBox <gi...@apache.org>.
SparkQA removed a comment on issue #28195: [SPARK-31425][SQL][CORE] UnsafeKVExternalSorter/VariableLengthRowBasedKeyValueBatch should also respect UnsafeAlignedOffset
URL: https://github.com/apache/spark/pull/28195#issuecomment-613560342
**[Test build #121284 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/121284/testReport)** for PR 28195 at commit [`bda5823`](https://github.com/apache/spark/commit/bda58233051227a679042abf77f859effbe29169).
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] jiangxb1987 commented on a change in pull request #28195:
[SPARK-31425][SQL][CORE]
UnsafeKVExternalSorter/VariableLengthRowBasedKeyValueBatch should also
respect UnsafeAlignedOffset
Posted by GitBox <gi...@apache.org>.
jiangxb1987 commented on a change in pull request #28195: [SPARK-31425][SQL][CORE] UnsafeKVExternalSorter/VariableLengthRowBasedKeyValueBatch should also respect UnsafeAlignedOffset
URL: https://github.com/apache/spark/pull/28195#discussion_r408608737
##########
File path: core/src/main/java/org/apache/spark/unsafe/map/BytesToBytesMap.java
##########
@@ -53,7 +53,8 @@
* The map can support up to 2^29 keys. If the key cardinality is higher than this, you should
* probably be using sorting instead of hashing for better cache locality.
*
- * The key and values under the hood are stored together, in the following format:
+ * The key and values under the hood are stored together, in the following format(in case of
+ * uaoSize = 4):
Review comment:
why not replace the number below with `uaoSize`?
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] Ngone51 commented on a change in pull request #28195:
[SPARK-31425][SQL][CORE]
UnsafeKVExternalSorter/VariableLengthRowBasedKeyValueBatch should also
respect UnsafeAlignedOffset
Posted by GitBox <gi...@apache.org>.
Ngone51 commented on a change in pull request #28195: [SPARK-31425][SQL][CORE] UnsafeKVExternalSorter/VariableLengthRowBasedKeyValueBatch should also respect UnsafeAlignedOffset
URL: https://github.com/apache/spark/pull/28195#discussion_r408616120
##########
File path: core/src/main/java/org/apache/spark/unsafe/map/BytesToBytesMap.java
##########
@@ -53,7 +53,8 @@
* The map can support up to 2^29 keys. If the key cardinality is higher than this, you should
* probably be using sorting instead of hashing for better cache locality.
*
- * The key and values under the hood are stored together, in the following format:
+ * The key and values under the hood are stored together, in the following format(in case of
+ * uaoSize = 4):
Review comment:
Ok. I'll update later.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA removed a comment on issue #28195:
[SPARK-31425][SQL][CORE] UnsafeKVExternalSorter should also respect
UnsafeAlignedOffset
Posted by GitBox <gi...@apache.org>.
SparkQA removed a comment on issue #28195: [SPARK-31425][SQL][CORE] UnsafeKVExternalSorter should also respect UnsafeAlignedOffset
URL: https://github.com/apache/spark/pull/28195#issuecomment-612629291
**[Test build #121147 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/121147/testReport)** for PR 28195 at commit [`6943425`](https://github.com/apache/spark/commit/694342549b99c15efce1410d782542d3e16e23a1).
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins removed a comment on issue #28195:
[SPARK-31425][SQL][CORE] UnsafeKVExternalSorter should also respect
UnsafeAlignedOffset
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on issue #28195: [SPARK-31425][SQL][CORE] UnsafeKVExternalSorter should also respect UnsafeAlignedOffset
URL: https://github.com/apache/spark/pull/28195#issuecomment-612888050
Merged build finished. Test FAILed.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA removed a comment on issue #28195:
[SPARK-31425][SQL][CORE]
UnsafeKVExternalSorter/VariableLengthRowBasedKeyValueBatch should also
respect UnsafeAlignedOffset
Posted by GitBox <gi...@apache.org>.
SparkQA removed a comment on issue #28195: [SPARK-31425][SQL][CORE] UnsafeKVExternalSorter/VariableLengthRowBasedKeyValueBatch should also respect UnsafeAlignedOffset
URL: https://github.com/apache/spark/pull/28195#issuecomment-614690691
**[Test build #121368 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/121368/testReport)** for PR 28195 at commit [`c6f39b5`](https://github.com/apache/spark/commit/c6f39b5a25e1e92c7f5a71ca541e9ce501850000).
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins commented on issue #28195:
[SPARK-31425][SQL][CORE]
UnsafeKVExternalSorter/VariableLengthRowBasedKeyValueBatch should also
respect UnsafeAlignedOffset
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on issue #28195: [SPARK-31425][SQL][CORE] UnsafeKVExternalSorter/VariableLengthRowBasedKeyValueBatch should also respect UnsafeAlignedOffset
URL: https://github.com/apache/spark/pull/28195#issuecomment-614789263
Merged build finished. Test PASSed.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] cloud-fan commented on a change in pull request #28195:
[SPARK-31425][SQL][CORE]
UnsafeKVExternalSorter/VariableLengthRowBasedKeyValueBatch should also
respect UnsafeAlignedOffset
Posted by GitBox <gi...@apache.org>.
cloud-fan commented on a change in pull request #28195: [SPARK-31425][SQL][CORE] UnsafeKVExternalSorter/VariableLengthRowBasedKeyValueBatch should also respect UnsafeAlignedOffset
URL: https://github.com/apache/spark/pull/28195#discussion_r407949638
##########
File path: core/src/main/java/org/apache/spark/unsafe/map/BytesToBytesMap.java
##########
@@ -53,7 +53,7 @@
* The map can support up to 2^29 keys. If the key cardinality is higher than this, you should
* probably be using sorting instead of hashing for better cache locality.
*
- * The key and values under the hood are stored together, in the following format:
+ * The key and values under the hood are stored together, in the following format(uaoSize = 4):
Review comment:
in case of uaoSize = 4
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] jiangxb1987 commented on a change in pull request #28195:
[SPARK-31425][SQL][CORE]
UnsafeKVExternalSorter/VariableLengthRowBasedKeyValueBatch should also
respect UnsafeAlignedOffset
Posted by GitBox <gi...@apache.org>.
jiangxb1987 commented on a change in pull request #28195: [SPARK-31425][SQL][CORE] UnsafeKVExternalSorter/VariableLengthRowBasedKeyValueBatch should also respect UnsafeAlignedOffset
URL: https://github.com/apache/spark/pull/28195#discussion_r408608737
##########
File path: core/src/main/java/org/apache/spark/unsafe/map/BytesToBytesMap.java
##########
@@ -53,7 +53,8 @@
* The map can support up to 2^29 keys. If the key cardinality is higher than this, you should
* probably be using sorting instead of hashing for better cache locality.
*
- * The key and values under the hood are stored together, in the following format:
+ * The key and values under the hood are stored together, in the following format(in case of
+ * uaoSize = 4):
Review comment:
why not replace the number with `uaoSize`?
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] jiangxb1987 commented on a change in pull request #28195:
[SPARK-31425][SQL][CORE]
UnsafeKVExternalSorter/VariableLengthRowBasedKeyValueBatch should also
respect UnsafeAlignedOffset
Posted by GitBox <gi...@apache.org>.
jiangxb1987 commented on a change in pull request #28195: [SPARK-31425][SQL][CORE] UnsafeKVExternalSorter/VariableLengthRowBasedKeyValueBatch should also respect UnsafeAlignedOffset
URL: https://github.com/apache/spark/pull/28195#discussion_r408608316
##########
File path: common/unsafe/src/main/java/org/apache/spark/unsafe/UnsafeAlignedOffset.java
##########
@@ -28,12 +28,20 @@
private static final int UAO_SIZE = Platform.unaligned() ? 4 : 8;
+ private static int TEST_UAO_SIZE = 0;
+
+ // used for test only
+ public static void setUaoSize(int size) {
+ assert size == 0 || size == 4 || size == 8;
+ TEST_UAO_SIZE = size;
+ }
+
public static int getUaoSize() {
- return UAO_SIZE;
+ return TEST_UAO_SIZE == 0 ? UAO_SIZE : TEST_UAO_SIZE;
Review comment:
nit: you can use `Utils.isTesting` instead
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] cloud-fan closed pull request #28195:
[SPARK-31425][SQL][CORE]
UnsafeKVExternalSorter/VariableLengthRowBasedKeyValueBatch should also
respect UnsafeAlignedOffset
Posted by GitBox <gi...@apache.org>.
cloud-fan closed pull request #28195: [SPARK-31425][SQL][CORE] UnsafeKVExternalSorter/VariableLengthRowBasedKeyValueBatch should also respect UnsafeAlignedOffset
URL: https://github.com/apache/spark/pull/28195
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins commented on issue #28195:
[SPARK-31425][SQL][CORE] UnsafeKVExternalSorter should also respect
UnsafeAlignedOffset
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on issue #28195: [SPARK-31425][SQL][CORE] UnsafeKVExternalSorter should also respect UnsafeAlignedOffset
URL: https://github.com/apache/spark/pull/28195#issuecomment-612649414
Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/121147/
Test PASSed.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] Ngone51 commented on a change in pull request #28195:
[SPARK-31425][SQL][CORE]
UnsafeKVExternalSorter/VariableLengthRowBasedKeyValueBatch should also
respect UnsafeAlignedOffset
Posted by GitBox <gi...@apache.org>.
Ngone51 commented on a change in pull request #28195: [SPARK-31425][SQL][CORE] UnsafeKVExternalSorter/VariableLengthRowBasedKeyValueBatch should also respect UnsafeAlignedOffset
URL: https://github.com/apache/spark/pull/28195#discussion_r408611757
##########
File path: core/src/main/java/org/apache/spark/unsafe/map/BytesToBytesMap.java
##########
@@ -53,7 +53,8 @@
* The map can support up to 2^29 keys. If the key cardinality is higher than this, you should
* probably be using sorting instead of hashing for better cache locality.
*
- * The key and values under the hood are stored together, in the following format:
+ * The key and values under the hood are stored together, in the following format(in case of
+ * uaoSize = 4):
Review comment:
I was supposed to do so but it requires more changes in the original content and it is not so clean compares to a concrete number. So I am not sure if we should do. Do you have strong prefer to change in `uaoSize` way?
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins removed a comment on issue #28195:
[SPARK-31425][SQL][CORE]
UnsafeKVExternalSorter/VariableLengthRowBasedKeyValueBatch should also
respect UnsafeAlignedOffset
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on issue #28195: [SPARK-31425][SQL][CORE] UnsafeKVExternalSorter/VariableLengthRowBasedKeyValueBatch should also respect UnsafeAlignedOffset
URL: https://github.com/apache/spark/pull/28195#issuecomment-614691389
Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder-K8s/26053/
Test PASSed.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins removed a comment on issue #28195:
[SPARK-31425][SQL][CORE]
UnsafeKVExternalSorter/VariableLengthRowBasedKeyValueBatch should also
respect UnsafeAlignedOffset
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on issue #28195: [SPARK-31425][SQL][CORE] UnsafeKVExternalSorter/VariableLengthRowBasedKeyValueBatch should also respect UnsafeAlignedOffset
URL: https://github.com/apache/spark/pull/28195#issuecomment-613532610
Merged build finished. Test FAILed.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins removed a comment on issue #28195:
[SPARK-31425][SQL][CORE]
UnsafeKVExternalSorter/VariableLengthRowBasedKeyValueBatch should also
respect UnsafeAlignedOffset
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on issue #28195: [SPARK-31425][SQL][CORE] UnsafeKVExternalSorter/VariableLengthRowBasedKeyValueBatch should also respect UnsafeAlignedOffset
URL: https://github.com/apache/spark/pull/28195#issuecomment-614015409
Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder-K8s/26007/
Test PASSed.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins removed a comment on issue #28195:
[SPARK-31425][SQL][CORE]
UnsafeKVExternalSorter/VariableLengthRowBasedKeyValueBatch should also
respect UnsafeAlignedOffset
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on issue #28195: [SPARK-31425][SQL][CORE] UnsafeKVExternalSorter/VariableLengthRowBasedKeyValueBatch should also respect UnsafeAlignedOffset
URL: https://github.com/apache/spark/pull/28195#issuecomment-613818634
Merged build finished. Test PASSed.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins commented on issue #28195:
[SPARK-31425][SQL][CORE]
UnsafeKVExternalSorter/VariableLengthRowBasedKeyValueBatch should also
respect UnsafeAlignedOffset
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on issue #28195: [SPARK-31425][SQL][CORE] UnsafeKVExternalSorter/VariableLengthRowBasedKeyValueBatch should also respect UnsafeAlignedOffset
URL: https://github.com/apache/spark/pull/28195#issuecomment-613368576
Merged build finished. Test FAILed.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins removed a comment on issue #28195:
[SPARK-31425][SQL][CORE]
UnsafeKVExternalSorter/VariableLengthRowBasedKeyValueBatch should also
respect UnsafeAlignedOffset
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on issue #28195: [SPARK-31425][SQL][CORE] UnsafeKVExternalSorter/VariableLengthRowBasedKeyValueBatch should also respect UnsafeAlignedOffset
URL: https://github.com/apache/spark/pull/28195#issuecomment-613414286
Merged build finished. Test PASSed.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins removed a comment on issue #28195:
[SPARK-31425][SQL][CORE]
UnsafeKVExternalSorter/VariableLengthRowBasedKeyValueBatch should also
respect UnsafeAlignedOffset
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on issue #28195: [SPARK-31425][SQL][CORE] UnsafeKVExternalSorter/VariableLengthRowBasedKeyValueBatch should also respect UnsafeAlignedOffset
URL: https://github.com/apache/spark/pull/28195#issuecomment-613561071
Merged build finished. Test PASSed.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins removed a comment on issue #28195:
[SPARK-31425][SQL][CORE]
UnsafeKVExternalSorter/VariableLengthRowBasedKeyValueBatch should also
respect UnsafeAlignedOffset
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on issue #28195: [SPARK-31425][SQL][CORE] UnsafeKVExternalSorter/VariableLengthRowBasedKeyValueBatch should also respect UnsafeAlignedOffset
URL: https://github.com/apache/spark/pull/28195#issuecomment-614691375
Merged build finished. Test PASSed.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA commented on issue #28195: [SPARK-31425][SQL][CORE]
UnsafeKVExternalSorter should also respect UnsafeAlignedOffset
Posted by GitBox <gi...@apache.org>.
SparkQA commented on issue #28195: [SPARK-31425][SQL][CORE] UnsafeKVExternalSorter should also respect UnsafeAlignedOffset
URL: https://github.com/apache/spark/pull/28195#issuecomment-612629291
**[Test build #121147 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/121147/testReport)** for PR 28195 at commit [`6943425`](https://github.com/apache/spark/commit/694342549b99c15efce1410d782542d3e16e23a1).
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins removed a comment on issue #28195:
[SPARK-31425][SQL][CORE]
UnsafeKVExternalSorter/VariableLengthRowBasedKeyValueBatch should also
respect UnsafeAlignedOffset
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on issue #28195: [SPARK-31425][SQL][CORE] UnsafeKVExternalSorter/VariableLengthRowBasedKeyValueBatch should also respect UnsafeAlignedOffset
URL: https://github.com/apache/spark/pull/28195#issuecomment-613644266
Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/121284/
Test PASSed.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins commented on issue #28195:
[SPARK-31425][SQL][CORE]
UnsafeKVExternalSorter/VariableLengthRowBasedKeyValueBatch should also
respect UnsafeAlignedOffset
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on issue #28195: [SPARK-31425][SQL][CORE] UnsafeKVExternalSorter/VariableLengthRowBasedKeyValueBatch should also respect UnsafeAlignedOffset
URL: https://github.com/apache/spark/pull/28195#issuecomment-614015409
Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder-K8s/26007/
Test PASSed.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins commented on issue #28195:
[SPARK-31425][SQL][CORE] UnsafeKVExternalSorter should also respect
UnsafeAlignedOffset
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on issue #28195: [SPARK-31425][SQL][CORE] UnsafeKVExternalSorter should also respect UnsafeAlignedOffset
URL: https://github.com/apache/spark/pull/28195#issuecomment-613277934
Merged build finished. Test PASSed.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins removed a comment on issue #28195:
[SPARK-31425][SQL][CORE]
UnsafeKVExternalSorter/VariableLengthRowBasedKeyValueBatch should also
respect UnsafeAlignedOffset
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on issue #28195: [SPARK-31425][SQL][CORE] UnsafeKVExternalSorter/VariableLengthRowBasedKeyValueBatch should also respect UnsafeAlignedOffset
URL: https://github.com/apache/spark/pull/28195#issuecomment-613368584
Test FAILed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/121262/
Test FAILed.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA commented on issue #28195: [SPARK-31425][SQL][CORE]
UnsafeKVExternalSorter should also respect UnsafeAlignedOffset
Posted by GitBox <gi...@apache.org>.
SparkQA commented on issue #28195: [SPARK-31425][SQL][CORE] UnsafeKVExternalSorter should also respect UnsafeAlignedOffset
URL: https://github.com/apache/spark/pull/28195#issuecomment-613277335
**[Test build #121262 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/121262/testReport)** for PR 28195 at commit [`b1be35e`](https://github.com/apache/spark/commit/b1be35ec702f2caaf15a382a4e4aa80a32fac716).
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] Ngone51 commented on a change in pull request #28195:
[SPARK-31425][SQL][CORE]
UnsafeKVExternalSorter/VariableLengthRowBasedKeyValueBatch should also
respect UnsafeAlignedOffset
Posted by GitBox <gi...@apache.org>.
Ngone51 commented on a change in pull request #28195: [SPARK-31425][SQL][CORE] UnsafeKVExternalSorter/VariableLengthRowBasedKeyValueBatch should also respect UnsafeAlignedOffset
URL: https://github.com/apache/spark/pull/28195#discussion_r408534911
##########
File path: sql/hive/src/test/scala/org/apache/spark/sql/hive/execution/AggregationQuerySuite.scala
##########
@@ -1055,28 +1056,31 @@ class HashAggregationQueryWithControlledFallbackSuite extends AggregationQuerySu
Seq("true", "false").foreach { enableTwoLevelMaps =>
withSQLConf(SQLConf.ENABLE_TWOLEVEL_AGG_MAP.key ->
enableTwoLevelMaps) {
- (1 to 3).foreach { fallbackStartsAt =>
- withSQLConf("spark.sql.TungstenAggregate.testFallbackStartsAt" ->
- s"${(fallbackStartsAt - 1).toString}, ${fallbackStartsAt.toString}") {
- // Create a new df to make sure its physical operator picks up
- // spark.sql.TungstenAggregate.testFallbackStartsAt.
- // todo: remove it?
- val newActual = Dataset.ofRows(spark, actual.logicalPlan)
-
- QueryTest.getErrorMessageInCheckAnswer(newActual, expectedAnswer) match {
- case Some(errorMessage) =>
- val newErrorMessage =
- s"""
- |The following aggregation query failed when using HashAggregate with
- |controlled fallback (it falls back to bytes to bytes map once it has processed
- |${fallbackStartsAt - 1} input rows and to sort-based aggregation once it has
- |processed $fallbackStartsAt input rows). The query is ${actual.queryExecution}
- |
- |$errorMessage
- """.stripMargin
-
- fail(newErrorMessage)
- case None => // Success
+ Seq(4, 8).foreach { uaoSize =>
+ UnsafeAlignedOffset.setUaoSize(uaoSize)
+ (1 to 3).foreach { fallbackStartsAt =>
+ withSQLConf("spark.sql.TungstenAggregate.testFallbackStartsAt" ->
+ s"${(fallbackStartsAt - 1).toString}, ${fallbackStartsAt.toString}") {
+ // Create a new df to make sure its physical operator picks up
+ // spark.sql.TungstenAggregate.testFallbackStartsAt.
+ // todo: remove it?
+ val newActual = Dataset.ofRows(spark, actual.logicalPlan)
+
+ QueryTest.getErrorMessageInCheckAnswer(newActual, expectedAnswer) match {
+ case Some(errorMessage) =>
+ val newErrorMessage =
+ s"""
+ |The following aggregation query failed when using HashAggregate with
+ |controlled fallback (it falls back to bytes to bytes map once it has
+ |processed ${fallbackStartsAt - 1} input rows and to sort-based aggregation
+ |once it has processed $fallbackStartsAt input rows).
+ |The query is ${actual.queryExecution}
+ |$errorMessage
+ """.stripMargin
+
+ fail(newErrorMessage)
+ case None => // Success
+ }
Review comment:
Oh, yeah. I think we have to reset it.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins commented on issue #28195:
[SPARK-31425][SQL][CORE] UnsafeKVExternalSorter should also respect
UnsafeAlignedOffset
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on issue #28195: [SPARK-31425][SQL][CORE] UnsafeKVExternalSorter should also respect UnsafeAlignedOffset
URL: https://github.com/apache/spark/pull/28195#issuecomment-612628244
Merged build finished. Test PASSed.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins commented on issue #28195:
[SPARK-31425][SQL][CORE]
UnsafeKVExternalSorter/VariableLengthRowBasedKeyValueBatch should also
respect UnsafeAlignedOffset
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on issue #28195: [SPARK-31425][SQL][CORE] UnsafeKVExternalSorter/VariableLengthRowBasedKeyValueBatch should also respect UnsafeAlignedOffset
URL: https://github.com/apache/spark/pull/28195#issuecomment-613414286
Merged build finished. Test PASSed.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA removed a comment on issue #28195:
[SPARK-31425][SQL][CORE] UnsafeKVExternalSorter should also respect
UnsafeAlignedOffset
Posted by GitBox <gi...@apache.org>.
SparkQA removed a comment on issue #28195: [SPARK-31425][SQL][CORE] UnsafeKVExternalSorter should also respect UnsafeAlignedOffset
URL: https://github.com/apache/spark/pull/28195#issuecomment-612834121
**[Test build #121195 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/121195/testReport)** for PR 28195 at commit [`0b8ebd3`](https://github.com/apache/spark/commit/0b8ebd3abcc54e75ad0b9835a178ef95f66c8f3f).
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins removed a comment on issue #28195:
[SPARK-31425][SQL][CORE]
UnsafeKVExternalSorter/VariableLengthRowBasedKeyValueBatch should also
respect UnsafeAlignedOffset
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on issue #28195: [SPARK-31425][SQL][CORE] UnsafeKVExternalSorter/VariableLengthRowBasedKeyValueBatch should also respect UnsafeAlignedOffset
URL: https://github.com/apache/spark/pull/28195#issuecomment-614119846
Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/121324/
Test PASSed.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] Ngone51 commented on a change in pull request #28195:
[SPARK-31425][SQL][CORE]
UnsafeKVExternalSorter/VariableLengthRowBasedKeyValueBatch should also
respect UnsafeAlignedOffset
Posted by GitBox <gi...@apache.org>.
Ngone51 commented on a change in pull request #28195: [SPARK-31425][SQL][CORE] UnsafeKVExternalSorter/VariableLengthRowBasedKeyValueBatch should also respect UnsafeAlignedOffset
URL: https://github.com/apache/spark/pull/28195#discussion_r408614500
##########
File path: sql/core/src/main/java/org/apache/spark/sql/execution/UnsafeKVExternalSorter.java
##########
@@ -141,9 +142,10 @@ public UnsafeKVExternalSorter(
// Get encoded memory address
// baseObject + baseOffset point to the beginning of the key data in the map, but that
- // the KV-pair's length data is stored in the word immediately before that address
+ // the KV-pair's length data is stored at 2 * uaoSize bytes immediately before that address
Review comment:
The record format is:
```
(total length) (key length) (key) (value) (8 byte pointer to next value)
| |
uaoSize uaoSize
```
And we now get keyOffset, so we need back walk for 2 usao sizes to get the address.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins commented on issue #28195:
[SPARK-31425][SQL][CORE] UnsafeKVExternalSorter should also respect
UnsafeAlignedOffset
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on issue #28195: [SPARK-31425][SQL][CORE] UnsafeKVExternalSorter should also respect UnsafeAlignedOffset
URL: https://github.com/apache/spark/pull/28195#issuecomment-613277944
Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder-K8s/25949/
Test PASSed.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA commented on issue #28195: [SPARK-31425][SQL][CORE]
UnsafeKVExternalSorter should also respect UnsafeAlignedOffset
Posted by GitBox <gi...@apache.org>.
SparkQA commented on issue #28195: [SPARK-31425][SQL][CORE] UnsafeKVExternalSorter should also respect UnsafeAlignedOffset
URL: https://github.com/apache/spark/pull/28195#issuecomment-612649156
**[Test build #121147 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/121147/testReport)** for PR 28195 at commit [`6943425`](https://github.com/apache/spark/commit/694342549b99c15efce1410d782542d3e16e23a1).
* This patch passes all tests.
* This patch merges cleanly.
* This patch adds no public classes.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] jiangxb1987 commented on a change in pull request #28195:
[SPARK-31425][SQL][CORE]
UnsafeKVExternalSorter/VariableLengthRowBasedKeyValueBatch should also
respect UnsafeAlignedOffset
Posted by GitBox <gi...@apache.org>.
jiangxb1987 commented on a change in pull request #28195: [SPARK-31425][SQL][CORE] UnsafeKVExternalSorter/VariableLengthRowBasedKeyValueBatch should also respect UnsafeAlignedOffset
URL: https://github.com/apache/spark/pull/28195#discussion_r408611724
##########
File path: common/unsafe/src/main/java/org/apache/spark/unsafe/UnsafeAlignedOffset.java
##########
@@ -28,12 +28,20 @@
private static final int UAO_SIZE = Platform.unaligned() ? 4 : 8;
+ private static int TEST_UAO_SIZE = 0;
+
+ // used for test only
+ public static void setUaoSize(int size) {
+ assert size == 0 || size == 4 || size == 8;
+ TEST_UAO_SIZE = size;
+ }
+
public static int getUaoSize() {
- return UAO_SIZE;
+ return TEST_UAO_SIZE == 0 ? UAO_SIZE : TEST_UAO_SIZE;
Review comment:
We should handle the logic of setting/reverting the size in the test code.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA commented on issue #28195: [SPARK-31425][SQL][CORE]
UnsafeKVExternalSorter should also respect UnsafeAlignedOffset
Posted by GitBox <gi...@apache.org>.
SparkQA commented on issue #28195: [SPARK-31425][SQL][CORE] UnsafeKVExternalSorter should also respect UnsafeAlignedOffset
URL: https://github.com/apache/spark/pull/28195#issuecomment-612887612
**[Test build #121195 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/121195/testReport)** for PR 28195 at commit [`0b8ebd3`](https://github.com/apache/spark/commit/0b8ebd3abcc54e75ad0b9835a178ef95f66c8f3f).
* This patch **fails Spark unit tests**.
* This patch merges cleanly.
* This patch adds no public classes.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins commented on issue #28195:
[SPARK-31425][SQL][CORE]
UnsafeKVExternalSorter/VariableLengthRowBasedKeyValueBatch should also
respect UnsafeAlignedOffset
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on issue #28195: [SPARK-31425][SQL][CORE] UnsafeKVExternalSorter/VariableLengthRowBasedKeyValueBatch should also respect UnsafeAlignedOffset
URL: https://github.com/apache/spark/pull/28195#issuecomment-614789273
Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/121368/
Test PASSed.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins commented on issue #28195:
[SPARK-31425][SQL][CORE]
UnsafeKVExternalSorter/VariableLengthRowBasedKeyValueBatch should also
respect UnsafeAlignedOffset
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on issue #28195: [SPARK-31425][SQL][CORE] UnsafeKVExternalSorter/VariableLengthRowBasedKeyValueBatch should also respect UnsafeAlignedOffset
URL: https://github.com/apache/spark/pull/28195#issuecomment-613561082
Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder-K8s/25970/
Test PASSed.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] Ngone51 edited a comment on issue #28195:
[SPARK-31425][SQL][CORE] UnsafeKVExternalSorter should also respect
UnsafeAlignedOffset
Posted by GitBox <gi...@apache.org>.
Ngone51 edited a comment on issue #28195: [SPARK-31425][SQL][CORE] UnsafeKVExternalSorter should also respect UnsafeAlignedOffset
URL: https://github.com/apache/spark/pull/28195#issuecomment-612833274
> can we have a test?
yet, I updated `HashAggregationQueryWithControlledFallbackSuite` only to test both 4 and 8 `uaoSize`, which actually can fail with `uaoSize = 8`.
Seems better if we could update all those related tests to test both as well.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] Ngone51 commented on issue #28195: [SPARK-31425][SQL][CORE]
UnsafeKVExternalSorter should also respect UnsafeAlignedOffset
Posted by GitBox <gi...@apache.org>.
Ngone51 commented on issue #28195: [SPARK-31425][SQL][CORE] UnsafeKVExternalSorter should also respect UnsafeAlignedOffset
URL: https://github.com/apache/spark/pull/28195#issuecomment-612828414
No, I'll check them later and fix together if needed.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] cloud-fan commented on a change in pull request #28195:
[SPARK-31425][SQL][CORE] UnsafeKVExternalSorter should also respect
UnsafeAlignedOffset
Posted by GitBox <gi...@apache.org>.
cloud-fan commented on a change in pull request #28195: [SPARK-31425][SQL][CORE] UnsafeKVExternalSorter should also respect UnsafeAlignedOffset
URL: https://github.com/apache/spark/pull/28195#discussion_r407515536
##########
File path: core/src/main/java/org/apache/spark/unsafe/map/BytesToBytesMap.java
##########
@@ -706,7 +706,7 @@ public boolean append(Object kbase, long koff, int klen, Object vbase, long voff
// Here, we'll copy the data into our data pages. Because we only store a relative offset from
// the key address instead of storing the absolute address of the value, the key and value
// must be stored in the same memory page.
- // (8 byte key length) (key) (value) (8 byte pointer to next value)
+ // (uao: klen + vlen + uaolen) (uao: klen) (key) (value) (8 byte pointer to next value)
Review comment:
SGTM
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins removed a comment on issue #28195:
[SPARK-31425][SQL][CORE] UnsafeKVExternalSorter should also respect
UnsafeAlignedOffset
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on issue #28195: [SPARK-31425][SQL][CORE] UnsafeKVExternalSorter should also respect UnsafeAlignedOffset
URL: https://github.com/apache/spark/pull/28195#issuecomment-612649410
Merged build finished. Test PASSed.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] jiangxb1987 commented on a change in pull request #28195:
[SPARK-31425][SQL][CORE]
UnsafeKVExternalSorter/VariableLengthRowBasedKeyValueBatch should also
respect UnsafeAlignedOffset
Posted by GitBox <gi...@apache.org>.
jiangxb1987 commented on a change in pull request #28195: [SPARK-31425][SQL][CORE] UnsafeKVExternalSorter/VariableLengthRowBasedKeyValueBatch should also respect UnsafeAlignedOffset
URL: https://github.com/apache/spark/pull/28195#discussion_r408610853
##########
File path: sql/core/src/main/java/org/apache/spark/sql/execution/UnsafeKVExternalSorter.java
##########
@@ -141,9 +142,10 @@ public UnsafeKVExternalSorter(
// Get encoded memory address
// baseObject + baseOffset point to the beginning of the key data in the map, but that
- // the KV-pair's length data is stored in the word immediately before that address
+ // the KV-pair's length data is stored at 2 * uaoSize bytes immediately before that address
Review comment:
sorry I don't get it here why the address is related to uaoSize?
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins removed a comment on issue #28195:
[SPARK-31425][SQL][CORE] UnsafeKVExternalSorter should also respect
UnsafeAlignedOffset
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on issue #28195: [SPARK-31425][SQL][CORE] UnsafeKVExternalSorter should also respect UnsafeAlignedOffset
URL: https://github.com/apache/spark/pull/28195#issuecomment-612834492
Merged build finished. Test PASSed.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins removed a comment on issue #28195:
[SPARK-31425][SQL][CORE] UnsafeKVExternalSorter should also respect
UnsafeAlignedOffset
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on issue #28195: [SPARK-31425][SQL][CORE] UnsafeKVExternalSorter should also respect UnsafeAlignedOffset
URL: https://github.com/apache/spark/pull/28195#issuecomment-612628244
Merged build finished. Test PASSed.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins commented on issue #28195:
[SPARK-31425][SQL][CORE] UnsafeKVExternalSorter should also respect
UnsafeAlignedOffset
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on issue #28195: [SPARK-31425][SQL][CORE] UnsafeKVExternalSorter should also respect UnsafeAlignedOffset
URL: https://github.com/apache/spark/pull/28195#issuecomment-612888050
Merged build finished. Test FAILed.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins commented on issue #28195:
[SPARK-31425][SQL][CORE] UnsafeKVExternalSorter should also respect
UnsafeAlignedOffset
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on issue #28195: [SPARK-31425][SQL][CORE] UnsafeKVExternalSorter should also respect UnsafeAlignedOffset
URL: https://github.com/apache/spark/pull/28195#issuecomment-612888057
Test FAILed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/121195/
Test FAILed.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins commented on issue #28195:
[SPARK-31425][SQL][CORE]
UnsafeKVExternalSorter/VariableLengthRowBasedKeyValueBatch should also
respect UnsafeAlignedOffset
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on issue #28195: [SPARK-31425][SQL][CORE] UnsafeKVExternalSorter/VariableLengthRowBasedKeyValueBatch should also respect UnsafeAlignedOffset
URL: https://github.com/apache/spark/pull/28195#issuecomment-614691375
Merged build finished. Test PASSed.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins commented on issue #28195:
[SPARK-31425][SQL][CORE]
UnsafeKVExternalSorter/VariableLengthRowBasedKeyValueBatch should also
respect UnsafeAlignedOffset
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on issue #28195: [SPARK-31425][SQL][CORE] UnsafeKVExternalSorter/VariableLengthRowBasedKeyValueBatch should also respect UnsafeAlignedOffset
URL: https://github.com/apache/spark/pull/28195#issuecomment-613818640
Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/121297/
Test PASSed.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] kiszk commented on a change in pull request #28195:
[SPARK-31425][SQL][CORE]
UnsafeKVExternalSorter/VariableLengthRowBasedKeyValueBatch should also
respect UnsafeAlignedOffset
Posted by GitBox <gi...@apache.org>.
kiszk commented on a change in pull request #28195: [SPARK-31425][SQL][CORE] UnsafeKVExternalSorter/VariableLengthRowBasedKeyValueBatch should also respect UnsafeAlignedOffset
URL: https://github.com/apache/spark/pull/28195#discussion_r408356089
##########
File path: sql/hive/src/test/scala/org/apache/spark/sql/hive/execution/AggregationQuerySuite.scala
##########
@@ -1055,28 +1056,31 @@ class HashAggregationQueryWithControlledFallbackSuite extends AggregationQuerySu
Seq("true", "false").foreach { enableTwoLevelMaps =>
withSQLConf(SQLConf.ENABLE_TWOLEVEL_AGG_MAP.key ->
enableTwoLevelMaps) {
- (1 to 3).foreach { fallbackStartsAt =>
- withSQLConf("spark.sql.TungstenAggregate.testFallbackStartsAt" ->
- s"${(fallbackStartsAt - 1).toString}, ${fallbackStartsAt.toString}") {
- // Create a new df to make sure its physical operator picks up
- // spark.sql.TungstenAggregate.testFallbackStartsAt.
- // todo: remove it?
- val newActual = Dataset.ofRows(spark, actual.logicalPlan)
-
- QueryTest.getErrorMessageInCheckAnswer(newActual, expectedAnswer) match {
- case Some(errorMessage) =>
- val newErrorMessage =
- s"""
- |The following aggregation query failed when using HashAggregate with
- |controlled fallback (it falls back to bytes to bytes map once it has processed
- |${fallbackStartsAt - 1} input rows and to sort-based aggregation once it has
- |processed $fallbackStartsAt input rows). The query is ${actual.queryExecution}
- |
- |$errorMessage
- """.stripMargin
-
- fail(newErrorMessage)
- case None => // Success
+ Seq(4, 8).foreach { uaoSize =>
+ UnsafeAlignedOffset.setUaoSize(uaoSize)
+ (1 to 3).foreach { fallbackStartsAt =>
+ withSQLConf("spark.sql.TungstenAggregate.testFallbackStartsAt" ->
+ s"${(fallbackStartsAt - 1).toString}, ${fallbackStartsAt.toString}") {
+ // Create a new df to make sure its physical operator picks up
+ // spark.sql.TungstenAggregate.testFallbackStartsAt.
+ // todo: remove it?
+ val newActual = Dataset.ofRows(spark, actual.logicalPlan)
+
+ QueryTest.getErrorMessageInCheckAnswer(newActual, expectedAnswer) match {
+ case Some(errorMessage) =>
+ val newErrorMessage =
+ s"""
+ |The following aggregation query failed when using HashAggregate with
+ |controlled fallback (it falls back to bytes to bytes map once it has
+ |processed ${fallbackStartsAt - 1} input rows and to sort-based aggregation
+ |once it has processed $fallbackStartsAt input rows).
+ |The query is ${actual.queryExecution}
+ |$errorMessage
+ """.stripMargin
+
+ fail(newErrorMessage)
+ case None => // Success
+ }
Review comment:
I am not confident, but do we need to call `setUaoSize(0)` at the end of this test since `TEST_UAO_SIZE` is static.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] Ngone51 commented on a change in pull request #28195:
[SPARK-31425][SQL][CORE]
UnsafeKVExternalSorter/VariableLengthRowBasedKeyValueBatch should also
respect UnsafeAlignedOffset
Posted by GitBox <gi...@apache.org>.
Ngone51 commented on a change in pull request #28195: [SPARK-31425][SQL][CORE] UnsafeKVExternalSorter/VariableLengthRowBasedKeyValueBatch should also respect UnsafeAlignedOffset
URL: https://github.com/apache/spark/pull/28195#discussion_r408614500
##########
File path: sql/core/src/main/java/org/apache/spark/sql/execution/UnsafeKVExternalSorter.java
##########
@@ -141,9 +142,10 @@ public UnsafeKVExternalSorter(
// Get encoded memory address
// baseObject + baseOffset point to the beginning of the key data in the map, but that
- // the KV-pair's length data is stored in the word immediately before that address
+ // the KV-pair's length data is stored at 2 * uaoSize bytes immediately before that address
Review comment:
The record format is:
```
(total length) (key length) (key) (value) (8 byte pointer to next value)
| |
uaoSize uaoSize
```
And we now get keyOffset, so we need back walk for 2 usao sizes to get the address of the record.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] jiangxb1987 commented on a change in pull request #28195:
[SPARK-31425][SQL][CORE]
UnsafeKVExternalSorter/VariableLengthRowBasedKeyValueBatch should also
respect UnsafeAlignedOffset
Posted by GitBox <gi...@apache.org>.
jiangxb1987 commented on a change in pull request #28195: [SPARK-31425][SQL][CORE] UnsafeKVExternalSorter/VariableLengthRowBasedKeyValueBatch should also respect UnsafeAlignedOffset
URL: https://github.com/apache/spark/pull/28195#discussion_r408618027
##########
File path: sql/core/src/main/java/org/apache/spark/sql/execution/UnsafeKVExternalSorter.java
##########
@@ -141,9 +142,10 @@ public UnsafeKVExternalSorter(
// Get encoded memory address
// baseObject + baseOffset point to the beginning of the key data in the map, but that
- // the KV-pair's length data is stored in the word immediately before that address
+ // the KV-pair's length data is stored at 2 * uaoSize bytes immediately before that address
Review comment:
so you mean we need to add back 2 * uaoSize to cover the space of storing total length and key length? If that's the case then it makes sense.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] cloud-fan commented on a change in pull request #28195:
[SPARK-31425][SQL][CORE]
UnsafeKVExternalSorter/VariableLengthRowBasedKeyValueBatch should also
respect UnsafeAlignedOffset
Posted by GitBox <gi...@apache.org>.
cloud-fan commented on a change in pull request #28195: [SPARK-31425][SQL][CORE] UnsafeKVExternalSorter/VariableLengthRowBasedKeyValueBatch should also respect UnsafeAlignedOffset
URL: https://github.com/apache/spark/pull/28195#discussion_r407950682
##########
File path: sql/catalyst/src/main/java/org/apache/spark/sql/catalyst/expressions/VariableLengthRowBasedKeyValueBatch.java
##########
@@ -99,9 +102,10 @@ public UnsafeRow getValueFromKey(int rowId) {
getKeyRow(rowId);
}
assert(rowId >= 0);
+ int uaoSize = UnsafeAlignedOffset.getUaoSize();
long offset = keyRow.getBaseOffset();
int klen = keyRow.getSizeInBytes();
- int vlen = Platform.getInt(base, offset - 8) - klen - 4;
+ int vlen = UnsafeAlignedOffset.getSize(base, offset - uaoSize - uaoSize) - klen - uaoSize;
Review comment:
`- uaoSize * 2`
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins commented on issue #28195:
[SPARK-31425][SQL][CORE]
UnsafeKVExternalSorter/VariableLengthRowBasedKeyValueBatch should also
respect UnsafeAlignedOffset
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on issue #28195: [SPARK-31425][SQL][CORE] UnsafeKVExternalSorter/VariableLengthRowBasedKeyValueBatch should also respect UnsafeAlignedOffset
URL: https://github.com/apache/spark/pull/28195#issuecomment-613644257
Merged build finished. Test PASSed.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins commented on issue #28195:
[SPARK-31425][SQL][CORE]
UnsafeKVExternalSorter/VariableLengthRowBasedKeyValueBatch should also
respect UnsafeAlignedOffset
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on issue #28195: [SPARK-31425][SQL][CORE] UnsafeKVExternalSorter/VariableLengthRowBasedKeyValueBatch should also respect UnsafeAlignedOffset
URL: https://github.com/apache/spark/pull/28195#issuecomment-613772065
Merged build finished. Test PASSed.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] Ngone51 edited a comment on issue #28195:
[SPARK-31425][SQL][CORE] UnsafeKVExternalSorter should also respect
UnsafeAlignedOffset
Posted by GitBox <gi...@apache.org>.
Ngone51 edited a comment on issue #28195: [SPARK-31425][SQL][CORE] UnsafeKVExternalSorter should also respect UnsafeAlignedOffset
URL: https://github.com/apache/spark/pull/28195#issuecomment-612833274
> can we have a test?
yet, I updated `HashAggregationQueryWithControlledFallbackSuite` to test both 4 and 8 `uaoSize`, which actually can fail with `uaoSize = 8`.
Seems better if we could update all those related tests to test both as well.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins commented on issue #28195:
[SPARK-31425][SQL][CORE]
UnsafeKVExternalSorter/VariableLengthRowBasedKeyValueBatch should also
respect UnsafeAlignedOffset
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on issue #28195: [SPARK-31425][SQL][CORE] UnsafeKVExternalSorter/VariableLengthRowBasedKeyValueBatch should also respect UnsafeAlignedOffset
URL: https://github.com/apache/spark/pull/28195#issuecomment-613644266
Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/121284/
Test PASSed.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA commented on issue #28195: [SPARK-31425][SQL][CORE]
UnsafeKVExternalSorter/VariableLengthRowBasedKeyValueBatch should also
respect UnsafeAlignedOffset
Posted by GitBox <gi...@apache.org>.
SparkQA commented on issue #28195: [SPARK-31425][SQL][CORE] UnsafeKVExternalSorter/VariableLengthRowBasedKeyValueBatch should also respect UnsafeAlignedOffset
URL: https://github.com/apache/spark/pull/28195#issuecomment-613773713
**[Test build #121297 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/121297/testReport)** for PR 28195 at commit [`d441e53`](https://github.com/apache/spark/commit/d441e5375859d2339334e7fd431b3c635407c516).
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins removed a comment on issue #28195:
[SPARK-31425][SQL][CORE]
UnsafeKVExternalSorter/VariableLengthRowBasedKeyValueBatch should also
respect UnsafeAlignedOffset
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on issue #28195: [SPARK-31425][SQL][CORE] UnsafeKVExternalSorter/VariableLengthRowBasedKeyValueBatch should also respect UnsafeAlignedOffset
URL: https://github.com/apache/spark/pull/28195#issuecomment-614789273
Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/121368/
Test PASSed.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins removed a comment on issue #28195:
[SPARK-31425][SQL][CORE] UnsafeKVExternalSorter should also respect
UnsafeAlignedOffset
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on issue #28195: [SPARK-31425][SQL][CORE] UnsafeKVExternalSorter should also respect UnsafeAlignedOffset
URL: https://github.com/apache/spark/pull/28195#issuecomment-613277944
Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder-K8s/25949/
Test PASSed.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins commented on issue #28195:
[SPARK-31425][SQL][CORE]
UnsafeKVExternalSorter/VariableLengthRowBasedKeyValueBatch should also
respect UnsafeAlignedOffset
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on issue #28195: [SPARK-31425][SQL][CORE] UnsafeKVExternalSorter/VariableLengthRowBasedKeyValueBatch should also respect UnsafeAlignedOffset
URL: https://github.com/apache/spark/pull/28195#issuecomment-613818634
Merged build finished. Test PASSed.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins commented on issue #28195:
[SPARK-31425][SQL][CORE]
UnsafeKVExternalSorter/VariableLengthRowBasedKeyValueBatch should also
respect UnsafeAlignedOffset
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on issue #28195: [SPARK-31425][SQL][CORE] UnsafeKVExternalSorter/VariableLengthRowBasedKeyValueBatch should also respect UnsafeAlignedOffset
URL: https://github.com/apache/spark/pull/28195#issuecomment-613561071
Merged build finished. Test PASSed.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] Ngone51 commented on issue #28195: [SPARK-31425][SQL][CORE]
UnsafeKVExternalSorter/VariableLengthRowBasedKeyValueBatch should also
respect UnsafeAlignedOffset
Posted by GitBox <gi...@apache.org>.
Ngone51 commented on issue #28195: [SPARK-31425][SQL][CORE] UnsafeKVExternalSorter/VariableLengthRowBasedKeyValueBatch should also respect UnsafeAlignedOffset
URL: https://github.com/apache/spark/pull/28195#issuecomment-613559439
retest this please.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] Ngone51 commented on a change in pull request #28195:
[SPARK-31425][SQL][CORE]
UnsafeKVExternalSorter/VariableLengthRowBasedKeyValueBatch should also
respect UnsafeAlignedOffset
Posted by GitBox <gi...@apache.org>.
Ngone51 commented on a change in pull request #28195: [SPARK-31425][SQL][CORE] UnsafeKVExternalSorter/VariableLengthRowBasedKeyValueBatch should also respect UnsafeAlignedOffset
URL: https://github.com/apache/spark/pull/28195#discussion_r408612502
##########
File path: common/unsafe/src/main/java/org/apache/spark/unsafe/UnsafeAlignedOffset.java
##########
@@ -28,12 +28,20 @@
private static final int UAO_SIZE = Platform.unaligned() ? 4 : 8;
+ private static int TEST_UAO_SIZE = 0;
+
+ // used for test only
+ public static void setUaoSize(int size) {
+ assert size == 0 || size == 4 || size == 8;
+ TEST_UAO_SIZE = size;
+ }
+
public static int getUaoSize() {
- return UAO_SIZE;
+ return TEST_UAO_SIZE == 0 ? UAO_SIZE : TEST_UAO_SIZE;
Review comment:
then, we need to figure out all the test cases where used `UnsafeAlignedOffset`, right?
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins commented on issue #28195:
[SPARK-31425][SQL][CORE]
UnsafeKVExternalSorter/VariableLengthRowBasedKeyValueBatch should also
respect UnsafeAlignedOffset
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on issue #28195: [SPARK-31425][SQL][CORE] UnsafeKVExternalSorter/VariableLengthRowBasedKeyValueBatch should also respect UnsafeAlignedOffset
URL: https://github.com/apache/spark/pull/28195#issuecomment-613414292
Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder-K8s/25963/
Test PASSed.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins removed a comment on issue #28195:
[SPARK-31425][SQL][CORE]
UnsafeKVExternalSorter/VariableLengthRowBasedKeyValueBatch should also
respect UnsafeAlignedOffset
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on issue #28195: [SPARK-31425][SQL][CORE] UnsafeKVExternalSorter/VariableLengthRowBasedKeyValueBatch should also respect UnsafeAlignedOffset
URL: https://github.com/apache/spark/pull/28195#issuecomment-613772065
Merged build finished. Test PASSed.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins commented on issue #28195:
[SPARK-31425][SQL][CORE] UnsafeKVExternalSorter should also respect
UnsafeAlignedOffset
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on issue #28195: [SPARK-31425][SQL][CORE] UnsafeKVExternalSorter should also respect UnsafeAlignedOffset
URL: https://github.com/apache/spark/pull/28195#issuecomment-612649410
Merged build finished. Test PASSed.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins removed a comment on issue #28195:
[SPARK-31425][SQL][CORE]
UnsafeKVExternalSorter/VariableLengthRowBasedKeyValueBatch should also
respect UnsafeAlignedOffset
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on issue #28195: [SPARK-31425][SQL][CORE] UnsafeKVExternalSorter/VariableLengthRowBasedKeyValueBatch should also respect UnsafeAlignedOffset
URL: https://github.com/apache/spark/pull/28195#issuecomment-614789263
Merged build finished. Test PASSed.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] cloud-fan commented on issue #28195:
[SPARK-31425][SQL][CORE] UnsafeKVExternalSorter should also respect
UnsafeAlignedOffset
Posted by GitBox <gi...@apache.org>.
cloud-fan commented on issue #28195: [SPARK-31425][SQL][CORE] UnsafeKVExternalSorter should also respect UnsafeAlignedOffset
URL: https://github.com/apache/spark/pull/28195#issuecomment-612755485
can we have a test?
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA commented on issue #28195: [SPARK-31425][SQL][CORE]
UnsafeKVExternalSorter/VariableLengthRowBasedKeyValueBatch should also
respect UnsafeAlignedOffset
Posted by GitBox <gi...@apache.org>.
SparkQA commented on issue #28195: [SPARK-31425][SQL][CORE] UnsafeKVExternalSorter/VariableLengthRowBasedKeyValueBatch should also respect UnsafeAlignedOffset
URL: https://github.com/apache/spark/pull/28195#issuecomment-613560342
**[Test build #121284 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/121284/testReport)** for PR 28195 at commit [`bda5823`](https://github.com/apache/spark/commit/bda58233051227a679042abf77f859effbe29169).
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins removed a comment on issue #28195:
[SPARK-31425][SQL][CORE]
UnsafeKVExternalSorter/VariableLengthRowBasedKeyValueBatch should also
respect UnsafeAlignedOffset
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on issue #28195: [SPARK-31425][SQL][CORE] UnsafeKVExternalSorter/VariableLengthRowBasedKeyValueBatch should also respect UnsafeAlignedOffset
URL: https://github.com/apache/spark/pull/28195#issuecomment-614119835
Merged build finished. Test PASSed.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins commented on issue #28195:
[SPARK-31425][SQL][CORE] UnsafeKVExternalSorter should also respect
UnsafeAlignedOffset
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on issue #28195: [SPARK-31425][SQL][CORE] UnsafeKVExternalSorter should also respect UnsafeAlignedOffset
URL: https://github.com/apache/spark/pull/28195#issuecomment-612628250
Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder-K8s/25834/
Test PASSed.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins commented on issue #28195:
[SPARK-31425][SQL][CORE]
UnsafeKVExternalSorter/VariableLengthRowBasedKeyValueBatch should also
respect UnsafeAlignedOffset
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on issue #28195: [SPARK-31425][SQL][CORE] UnsafeKVExternalSorter/VariableLengthRowBasedKeyValueBatch should also respect UnsafeAlignedOffset
URL: https://github.com/apache/spark/pull/28195#issuecomment-614119846
Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/121324/
Test PASSed.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins commented on issue #28195:
[SPARK-31425][SQL][CORE]
UnsafeKVExternalSorter/VariableLengthRowBasedKeyValueBatch should also
respect UnsafeAlignedOffset
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on issue #28195: [SPARK-31425][SQL][CORE] UnsafeKVExternalSorter/VariableLengthRowBasedKeyValueBatch should also respect UnsafeAlignedOffset
URL: https://github.com/apache/spark/pull/28195#issuecomment-614691389
Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder-K8s/26053/
Test PASSed.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins removed a comment on issue #28195:
[SPARK-31425][SQL][CORE] UnsafeKVExternalSorter should also respect
UnsafeAlignedOffset
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on issue #28195: [SPARK-31425][SQL][CORE] UnsafeKVExternalSorter should also respect UnsafeAlignedOffset
URL: https://github.com/apache/spark/pull/28195#issuecomment-612888057
Test FAILed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/121195/
Test FAILed.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] kiszk commented on a change in pull request #28195:
[SPARK-31425][SQL][CORE] UnsafeKVExternalSorter should also respect
UnsafeAlignedOffset
Posted by GitBox <gi...@apache.org>.
kiszk commented on a change in pull request #28195: [SPARK-31425][SQL][CORE] UnsafeKVExternalSorter should also respect UnsafeAlignedOffset
URL: https://github.com/apache/spark/pull/28195#discussion_r407406951
##########
File path: sql/hive/src/test/scala/org/apache/spark/sql/hive/execution/AggregationQuerySuite.scala
##########
@@ -1055,28 +1056,31 @@ class HashAggregationQueryWithControlledFallbackSuite extends AggregationQuerySu
Seq("true", "false").foreach { enableTwoLevelMaps =>
withSQLConf(SQLConf.ENABLE_TWOLEVEL_AGG_MAP.key ->
enableTwoLevelMaps) {
- (1 to 3).foreach { fallbackStartsAt =>
- withSQLConf("spark.sql.TungstenAggregate.testFallbackStartsAt" ->
- s"${(fallbackStartsAt - 1).toString}, ${fallbackStartsAt.toString}") {
- // Create a new df to make sure its physical operator picks up
- // spark.sql.TungstenAggregate.testFallbackStartsAt.
- // todo: remove it?
- val newActual = Dataset.ofRows(spark, actual.logicalPlan)
-
- QueryTest.getErrorMessageInCheckAnswer(newActual, expectedAnswer) match {
- case Some(errorMessage) =>
- val newErrorMessage =
- s"""
- |The following aggregation query failed when using HashAggregate with
- |controlled fallback (it falls back to bytes to bytes map once it has processed
- |${fallbackStartsAt - 1} input rows and to sort-based aggregation once it has
- |processed $fallbackStartsAt input rows). The query is ${actual.queryExecution}
- |
- |$errorMessage
+ Seq(4, 8).foreach { uaoSize =>
+ UnsafeAlignedOffset.setUaoSize(uaoSize)
+ (1 to 3).foreach { fallbackStartsAt =>
+ withSQLConf("spark.sql.TungstenAggregate.testFallbackStartsAt" ->
+ s"${(fallbackStartsAt - 1).toString}, ${fallbackStartsAt.toString}") {
+ // Create a new df to make sure its physical operator picks up
+ // spark.sql.TungstenAggregate.testFallbackStartsAt.
+ // todo: remove it?
+ val newActual = Dataset.ofRows(spark, actual.logicalPlan)
+
+ QueryTest.getErrorMessageInCheckAnswer(newActual, expectedAnswer) match {
+ case Some(errorMessage) =>
+ val newErrorMessage =
+ s"""
+ |The following aggregation query failed when using HashAggregate with
+ |controlled fallback (it falls back to bytes to bytes map once it has
+ |processed ${fallbackStartsAt - 1} input rows and to sort-based aggregation
+ |once it has processed $fallbackStartsAt input rows).
+ |The query is ${actual.queryExecution}
+ |$errorMessage
""".stripMargin
Review comment:
nit: indentation issue
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] kiszk commented on a change in pull request #28195:
[SPARK-31425][SQL][CORE]
UnsafeKVExternalSorter/VariableLengthRowBasedKeyValueBatch should also
respect UnsafeAlignedOffset
Posted by GitBox <gi...@apache.org>.
kiszk commented on a change in pull request #28195: [SPARK-31425][SQL][CORE] UnsafeKVExternalSorter/VariableLengthRowBasedKeyValueBatch should also respect UnsafeAlignedOffset
URL: https://github.com/apache/spark/pull/28195#discussion_r408356089
##########
File path: sql/hive/src/test/scala/org/apache/spark/sql/hive/execution/AggregationQuerySuite.scala
##########
@@ -1055,28 +1056,31 @@ class HashAggregationQueryWithControlledFallbackSuite extends AggregationQuerySu
Seq("true", "false").foreach { enableTwoLevelMaps =>
withSQLConf(SQLConf.ENABLE_TWOLEVEL_AGG_MAP.key ->
enableTwoLevelMaps) {
- (1 to 3).foreach { fallbackStartsAt =>
- withSQLConf("spark.sql.TungstenAggregate.testFallbackStartsAt" ->
- s"${(fallbackStartsAt - 1).toString}, ${fallbackStartsAt.toString}") {
- // Create a new df to make sure its physical operator picks up
- // spark.sql.TungstenAggregate.testFallbackStartsAt.
- // todo: remove it?
- val newActual = Dataset.ofRows(spark, actual.logicalPlan)
-
- QueryTest.getErrorMessageInCheckAnswer(newActual, expectedAnswer) match {
- case Some(errorMessage) =>
- val newErrorMessage =
- s"""
- |The following aggregation query failed when using HashAggregate with
- |controlled fallback (it falls back to bytes to bytes map once it has processed
- |${fallbackStartsAt - 1} input rows and to sort-based aggregation once it has
- |processed $fallbackStartsAt input rows). The query is ${actual.queryExecution}
- |
- |$errorMessage
- """.stripMargin
-
- fail(newErrorMessage)
- case None => // Success
+ Seq(4, 8).foreach { uaoSize =>
+ UnsafeAlignedOffset.setUaoSize(uaoSize)
+ (1 to 3).foreach { fallbackStartsAt =>
+ withSQLConf("spark.sql.TungstenAggregate.testFallbackStartsAt" ->
+ s"${(fallbackStartsAt - 1).toString}, ${fallbackStartsAt.toString}") {
+ // Create a new df to make sure its physical operator picks up
+ // spark.sql.TungstenAggregate.testFallbackStartsAt.
+ // todo: remove it?
+ val newActual = Dataset.ofRows(spark, actual.logicalPlan)
+
+ QueryTest.getErrorMessageInCheckAnswer(newActual, expectedAnswer) match {
+ case Some(errorMessage) =>
+ val newErrorMessage =
+ s"""
+ |The following aggregation query failed when using HashAggregate with
+ |controlled fallback (it falls back to bytes to bytes map once it has
+ |processed ${fallbackStartsAt - 1} input rows and to sort-based aggregation
+ |once it has processed $fallbackStartsAt input rows).
+ |The query is ${actual.queryExecution}
+ |$errorMessage
+ """.stripMargin
+
+ fail(newErrorMessage)
+ case None => // Success
+ }
Review comment:
I am not confident, but do we need to call `setUaoSize(0)` at the end of this test? This is because `TEST_UAO_SIZE` is static.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] Ngone51 commented on a change in pull request #28195:
[SPARK-31425][SQL][CORE]
UnsafeKVExternalSorter/VariableLengthRowBasedKeyValueBatch should also
respect UnsafeAlignedOffset
Posted by GitBox <gi...@apache.org>.
Ngone51 commented on a change in pull request #28195: [SPARK-31425][SQL][CORE] UnsafeKVExternalSorter/VariableLengthRowBasedKeyValueBatch should also respect UnsafeAlignedOffset
URL: https://github.com/apache/spark/pull/28195#discussion_r408628222
##########
File path: sql/core/src/main/java/org/apache/spark/sql/execution/UnsafeKVExternalSorter.java
##########
@@ -141,9 +142,10 @@ public UnsafeKVExternalSorter(
// Get encoded memory address
// baseObject + baseOffset point to the beginning of the key data in the map, but that
- // the KV-pair's length data is stored in the word immediately before that address
+ // the KV-pair's length data is stored at 2 * uaoSize bytes immediately before that address
Review comment:
yes.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] jiangxb1987 commented on a change in pull request #28195:
[SPARK-31425][SQL][CORE]
UnsafeKVExternalSorter/VariableLengthRowBasedKeyValueBatch should also
respect UnsafeAlignedOffset
Posted by GitBox <gi...@apache.org>.
jiangxb1987 commented on a change in pull request #28195: [SPARK-31425][SQL][CORE] UnsafeKVExternalSorter/VariableLengthRowBasedKeyValueBatch should also respect UnsafeAlignedOffset
URL: https://github.com/apache/spark/pull/28195#discussion_r408609505
##########
File path: sql/catalyst/src/main/java/org/apache/spark/sql/catalyst/expressions/VariableLengthRowBasedKeyValueBatch.java
##########
@@ -41,18 +42,20 @@
@Override
public UnsafeRow appendRow(Object kbase, long koff, int klen,
Object vbase, long voff, int vlen) {
- final long recordLength = 8L + klen + vlen + 8;
+ int uaoSize = UnsafeAlignedOffset.getUaoSize();
+ int doubleUaoSize = 2 * uaoSize;
Review comment:
`doubleUaoSize` sounds strange, I'd keep the 2 * uaoSize in the computation...
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] kiszk edited a comment on issue #28195:
[SPARK-31425][SQL][CORE] UnsafeKVExternalSorter should also respect
UnsafeAlignedOffset
Posted by GitBox <gi...@apache.org>.
kiszk edited a comment on issue #28195: [SPARK-31425][SQL][CORE] UnsafeKVExternalSorter should also respect UnsafeAlignedOffset
URL: https://github.com/apache/spark/pull/28195#issuecomment-612826485
@Ngone51 Did you run other tests in the environment at uao=8. Then, did they pass?
I am curious that the following files also depends on `uao=4`.
https://github.com/apache/spark/blob/master/sql/catalyst/src/main/java/org/apache/spark/sql/catalyst/expressions/VariableLengthRowBasedKeyValueBatch.java
https://github.com/apache/spark/blob/master/sql/catalyst/src/main/java/org/apache/spark/sql/catalyst/expressions/FixedLengthRowBasedKeyValueBatch.java
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] jiangxb1987 commented on a change in pull request #28195:
[SPARK-31425][SQL][CORE]
UnsafeKVExternalSorter/VariableLengthRowBasedKeyValueBatch should also
respect UnsafeAlignedOffset
Posted by GitBox <gi...@apache.org>.
jiangxb1987 commented on a change in pull request #28195: [SPARK-31425][SQL][CORE] UnsafeKVExternalSorter/VariableLengthRowBasedKeyValueBatch should also respect UnsafeAlignedOffset
URL: https://github.com/apache/spark/pull/28195#discussion_r408613304
##########
File path: common/unsafe/src/main/java/org/apache/spark/unsafe/UnsafeAlignedOffset.java
##########
@@ -28,12 +28,20 @@
private static final int UAO_SIZE = Platform.unaligned() ? 4 : 8;
+ private static int TEST_UAO_SIZE = 0;
+
+ // used for test only
+ public static void setUaoSize(int size) {
+ assert size == 0 || size == 4 || size == 8;
+ TEST_UAO_SIZE = size;
+ }
+
public static int getUaoSize() {
- return UAO_SIZE;
+ return TEST_UAO_SIZE == 0 ? UAO_SIZE : TEST_UAO_SIZE;
Review comment:
Why do we need to do that? The default value of TEST_UAO_SIZE should be the same as UAO_SIZE, only if you want to test other values then you need to change it.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA removed a comment on issue #28195:
[SPARK-31425][SQL][CORE]
UnsafeKVExternalSorter/VariableLengthRowBasedKeyValueBatch should also
respect UnsafeAlignedOffset
Posted by GitBox <gi...@apache.org>.
SparkQA removed a comment on issue #28195: [SPARK-31425][SQL][CORE] UnsafeKVExternalSorter/VariableLengthRowBasedKeyValueBatch should also respect UnsafeAlignedOffset
URL: https://github.com/apache/spark/pull/28195#issuecomment-614014662
**[Test build #121324 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/121324/testReport)** for PR 28195 at commit [`0fb6024`](https://github.com/apache/spark/commit/0fb6024e1b8007a295a69d4d4e99dd252a38cc9c).
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] Ngone51 commented on a change in pull request #28195:
[SPARK-31425][SQL][CORE]
UnsafeKVExternalSorter/VariableLengthRowBasedKeyValueBatch should also
respect UnsafeAlignedOffset
Posted by GitBox <gi...@apache.org>.
Ngone51 commented on a change in pull request #28195: [SPARK-31425][SQL][CORE] UnsafeKVExternalSorter/VariableLengthRowBasedKeyValueBatch should also respect UnsafeAlignedOffset
URL: https://github.com/apache/spark/pull/28195#discussion_r408808993
##########
File path: common/unsafe/src/main/java/org/apache/spark/unsafe/UnsafeAlignedOffset.java
##########
@@ -28,12 +28,20 @@
private static final int UAO_SIZE = Platform.unaligned() ? 4 : 8;
+ private static int TEST_UAO_SIZE = 0;
+
+ // used for test only
+ public static void setUaoSize(int size) {
+ assert size == 0 || size == 4 || size == 8;
+ TEST_UAO_SIZE = size;
+ }
+
public static int getUaoSize() {
- return UAO_SIZE;
+ return TEST_UAO_SIZE == 0 ? UAO_SIZE : TEST_UAO_SIZE;
Review comment:
hmm...unfortunately, `unsafe` project doesn't depend on `core`. So we can not access the `Utils`.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] cloud-fan commented on a change in pull request #28195:
[SPARK-31425][SQL][CORE]
UnsafeKVExternalSorter/VariableLengthRowBasedKeyValueBatch should also
respect UnsafeAlignedOffset
Posted by GitBox <gi...@apache.org>.
cloud-fan commented on a change in pull request #28195: [SPARK-31425][SQL][CORE] UnsafeKVExternalSorter/VariableLengthRowBasedKeyValueBatch should also respect UnsafeAlignedOffset
URL: https://github.com/apache/spark/pull/28195#discussion_r407951683
##########
File path: sql/hive/src/test/scala/org/apache/spark/sql/hive/execution/AggregationQuerySuite.scala
##########
@@ -1055,28 +1056,31 @@ class HashAggregationQueryWithControlledFallbackSuite extends AggregationQuerySu
Seq("true", "false").foreach { enableTwoLevelMaps =>
withSQLConf(SQLConf.ENABLE_TWOLEVEL_AGG_MAP.key ->
enableTwoLevelMaps) {
- (1 to 3).foreach { fallbackStartsAt =>
- withSQLConf("spark.sql.TungstenAggregate.testFallbackStartsAt" ->
- s"${(fallbackStartsAt - 1).toString}, ${fallbackStartsAt.toString}") {
- // Create a new df to make sure its physical operator picks up
- // spark.sql.TungstenAggregate.testFallbackStartsAt.
- // todo: remove it?
- val newActual = Dataset.ofRows(spark, actual.logicalPlan)
-
- QueryTest.getErrorMessageInCheckAnswer(newActual, expectedAnswer) match {
- case Some(errorMessage) =>
- val newErrorMessage =
- s"""
- |The following aggregation query failed when using HashAggregate with
- |controlled fallback (it falls back to bytes to bytes map once it has processed
- |${fallbackStartsAt - 1} input rows and to sort-based aggregation once it has
- |processed $fallbackStartsAt input rows). The query is ${actual.queryExecution}
- |
- |$errorMessage
+ Seq(4, 8).foreach { uaoSize =>
+ UnsafeAlignedOffset.setUaoSize(uaoSize)
+ (1 to 3).foreach { fallbackStartsAt =>
+ withSQLConf("spark.sql.TungstenAggregate.testFallbackStartsAt" ->
+ s"${(fallbackStartsAt - 1).toString}, ${fallbackStartsAt.toString}") {
+ // Create a new df to make sure its physical operator picks up
+ // spark.sql.TungstenAggregate.testFallbackStartsAt.
+ // todo: remove it?
+ val newActual = Dataset.ofRows(spark, actual.logicalPlan)
+
+ QueryTest.getErrorMessageInCheckAnswer(newActual, expectedAnswer) match {
+ case Some(errorMessage) =>
+ val newErrorMessage =
+ s"""
+ |The following aggregation query failed when using HashAggregate with
+ |controlled fallback (it falls back to bytes to bytes map once it has
+ |processed ${fallbackStartsAt - 1} input rows and to sort-based aggregation
+ |once it has processed $fallbackStartsAt input rows).
+ |The query is ${actual.queryExecution}
+ |$errorMessage
""".stripMargin
Review comment:
Let's fix it
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins commented on issue #28195:
[SPARK-31425][SQL][CORE]
UnsafeKVExternalSorter/VariableLengthRowBasedKeyValueBatch should also
respect UnsafeAlignedOffset
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on issue #28195: [SPARK-31425][SQL][CORE] UnsafeKVExternalSorter/VariableLengthRowBasedKeyValueBatch should also respect UnsafeAlignedOffset
URL: https://github.com/apache/spark/pull/28195#issuecomment-613772069
Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder-K8s/25981/
Test PASSed.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins commented on issue #28195:
[SPARK-31425][SQL][CORE] UnsafeKVExternalSorter should also respect
UnsafeAlignedOffset
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on issue #28195: [SPARK-31425][SQL][CORE] UnsafeKVExternalSorter should also respect UnsafeAlignedOffset
URL: https://github.com/apache/spark/pull/28195#issuecomment-612834492
Merged build finished. Test PASSed.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] Ngone51 commented on a change in pull request #28195:
[SPARK-31425][SQL][CORE] UnsafeKVExternalSorter should also respect
UnsafeAlignedOffset
Posted by GitBox <gi...@apache.org>.
Ngone51 commented on a change in pull request #28195: [SPARK-31425][SQL][CORE] UnsafeKVExternalSorter should also respect UnsafeAlignedOffset
URL: https://github.com/apache/spark/pull/28195#discussion_r407391851
##########
File path: core/src/main/java/org/apache/spark/unsafe/map/BytesToBytesMap.java
##########
@@ -706,7 +706,7 @@ public boolean append(Object kbase, long koff, int klen, Object vbase, long voff
// Here, we'll copy the data into our data pages. Because we only store a relative offset from
// the key address instead of storing the absolute address of the value, the key and value
// must be stored in the same memory page.
- // (8 byte key length) (key) (value) (8 byte pointer to next value)
+ // (uao: klen + vlen + uaolen) (uao: klen) (key) (value) (8 byte pointer to next value)
Review comment:
It will be a little bit redundant? Maybe just, `(total length) (key length) (key) (value) (8 byte pointer to next value)` ?
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA commented on issue #28195: [SPARK-31425][SQL][CORE]
UnsafeKVExternalSorter/VariableLengthRowBasedKeyValueBatch should also
respect UnsafeAlignedOffset
Posted by GitBox <gi...@apache.org>.
SparkQA commented on issue #28195: [SPARK-31425][SQL][CORE] UnsafeKVExternalSorter/VariableLengthRowBasedKeyValueBatch should also respect UnsafeAlignedOffset
URL: https://github.com/apache/spark/pull/28195#issuecomment-614690691
**[Test build #121368 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/121368/testReport)** for PR 28195 at commit [`c6f39b5`](https://github.com/apache/spark/commit/c6f39b5a25e1e92c7f5a71ca541e9ce501850000).
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] jiangxb1987 commented on a change in pull request #28195:
[SPARK-31425][SQL][CORE]
UnsafeKVExternalSorter/VariableLengthRowBasedKeyValueBatch should also
respect UnsafeAlignedOffset
Posted by GitBox <gi...@apache.org>.
jiangxb1987 commented on a change in pull request #28195: [SPARK-31425][SQL][CORE] UnsafeKVExternalSorter/VariableLengthRowBasedKeyValueBatch should also respect UnsafeAlignedOffset
URL: https://github.com/apache/spark/pull/28195#discussion_r408612355
##########
File path: core/src/main/java/org/apache/spark/unsafe/map/BytesToBytesMap.java
##########
@@ -53,7 +53,8 @@
* The map can support up to 2^29 keys. If the key cardinality is higher than this, you should
* probably be using sorting instead of hashing for better cache locality.
*
- * The key and values under the hood are stored together, in the following format:
+ * The key and values under the hood are stored together, in the following format(in case of
+ * uaoSize = 4):
Review comment:
The current way doesn't make it clear whether each number `4` represents the uaoSize.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins commented on issue #28195:
[SPARK-31425][SQL][CORE]
UnsafeKVExternalSorter/VariableLengthRowBasedKeyValueBatch should also
respect UnsafeAlignedOffset
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on issue #28195: [SPARK-31425][SQL][CORE] UnsafeKVExternalSorter/VariableLengthRowBasedKeyValueBatch should also respect UnsafeAlignedOffset
URL: https://github.com/apache/spark/pull/28195#issuecomment-614015397
Merged build finished. Test PASSed.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] kiszk commented on a change in pull request #28195:
[SPARK-31425][SQL][CORE] UnsafeKVExternalSorter should also respect
UnsafeAlignedOffset
Posted by GitBox <gi...@apache.org>.
kiszk commented on a change in pull request #28195: [SPARK-31425][SQL][CORE] UnsafeKVExternalSorter should also respect UnsafeAlignedOffset
URL: https://github.com/apache/spark/pull/28195#discussion_r407390812
##########
File path: core/src/main/java/org/apache/spark/unsafe/map/BytesToBytesMap.java
##########
@@ -706,7 +706,7 @@ public boolean append(Object kbase, long koff, int klen, Object vbase, long voff
// Here, we'll copy the data into our data pages. Because we only store a relative offset from
// the key address instead of storing the absolute address of the value, the key and value
// must be stored in the same memory page.
- // (8 byte key length) (key) (value) (8 byte pointer to next value)
+ // (uao: klen + vlen + uaolen) (uao: klen) (key) (value) (8 byte pointer to next value)
Review comment:
How about using the similar description at lines 56-61?
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA commented on issue #28195: [SPARK-31425][SQL][CORE]
UnsafeKVExternalSorter should also respect UnsafeAlignedOffset
Posted by GitBox <gi...@apache.org>.
SparkQA commented on issue #28195: [SPARK-31425][SQL][CORE] UnsafeKVExternalSorter should also respect UnsafeAlignedOffset
URL: https://github.com/apache/spark/pull/28195#issuecomment-612834121
**[Test build #121195 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/121195/testReport)** for PR 28195 at commit [`0b8ebd3`](https://github.com/apache/spark/commit/0b8ebd3abcc54e75ad0b9835a178ef95f66c8f3f).
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA commented on issue #28195: [SPARK-31425][SQL][CORE]
UnsafeKVExternalSorter/VariableLengthRowBasedKeyValueBatch should also
respect UnsafeAlignedOffset
Posted by GitBox <gi...@apache.org>.
SparkQA commented on issue #28195: [SPARK-31425][SQL][CORE] UnsafeKVExternalSorter/VariableLengthRowBasedKeyValueBatch should also respect UnsafeAlignedOffset
URL: https://github.com/apache/spark/pull/28195#issuecomment-614118575
**[Test build #121324 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/121324/testReport)** for PR 28195 at commit [`0fb6024`](https://github.com/apache/spark/commit/0fb6024e1b8007a295a69d4d4e99dd252a38cc9c).
* This patch passes all tests.
* This patch merges cleanly.
* This patch adds no public classes.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins removed a comment on issue #28195:
[SPARK-31425][SQL][CORE] UnsafeKVExternalSorter should also respect
UnsafeAlignedOffset
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on issue #28195: [SPARK-31425][SQL][CORE] UnsafeKVExternalSorter should also respect UnsafeAlignedOffset
URL: https://github.com/apache/spark/pull/28195#issuecomment-612834497
Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder-K8s/25884/
Test PASSed.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins removed a comment on issue #28195:
[SPARK-31425][SQL][CORE]
UnsafeKVExternalSorter/VariableLengthRowBasedKeyValueBatch should also
respect UnsafeAlignedOffset
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on issue #28195: [SPARK-31425][SQL][CORE] UnsafeKVExternalSorter/VariableLengthRowBasedKeyValueBatch should also respect UnsafeAlignedOffset
URL: https://github.com/apache/spark/pull/28195#issuecomment-613561082
Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder-K8s/25970/
Test PASSed.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] Ngone51 commented on issue #28195: [SPARK-31425][SQL][CORE]
UnsafeKVExternalSorter should also respect UnsafeAlignedOffset
Posted by GitBox <gi...@apache.org>.
Ngone51 commented on issue #28195: [SPARK-31425][SQL][CORE] UnsafeKVExternalSorter should also respect UnsafeAlignedOffset
URL: https://github.com/apache/spark/pull/28195#issuecomment-612711832
ping @cloud-fan @jiangxb1987 @kiszk Please take a look thanks!
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins commented on issue #28195:
[SPARK-31425][SQL][CORE] UnsafeKVExternalSorter should also respect
UnsafeAlignedOffset
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on issue #28195: [SPARK-31425][SQL][CORE] UnsafeKVExternalSorter should also respect UnsafeAlignedOffset
URL: https://github.com/apache/spark/pull/28195#issuecomment-612834497
Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder-K8s/25884/
Test PASSed.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA commented on issue #28195: [SPARK-31425][SQL][CORE]
UnsafeKVExternalSorter/VariableLengthRowBasedKeyValueBatch should also
respect UnsafeAlignedOffset
Posted by GitBox <gi...@apache.org>.
SparkQA commented on issue #28195: [SPARK-31425][SQL][CORE] UnsafeKVExternalSorter/VariableLengthRowBasedKeyValueBatch should also respect UnsafeAlignedOffset
URL: https://github.com/apache/spark/pull/28195#issuecomment-613413809
**[Test build #121276 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/121276/testReport)** for PR 28195 at commit [`bda5823`](https://github.com/apache/spark/commit/bda58233051227a679042abf77f859effbe29169).
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA commented on issue #28195: [SPARK-31425][SQL][CORE]
UnsafeKVExternalSorter/VariableLengthRowBasedKeyValueBatch should also
respect UnsafeAlignedOffset
Posted by GitBox <gi...@apache.org>.
SparkQA commented on issue #28195: [SPARK-31425][SQL][CORE] UnsafeKVExternalSorter/VariableLengthRowBasedKeyValueBatch should also respect UnsafeAlignedOffset
URL: https://github.com/apache/spark/pull/28195#issuecomment-613367466
**[Test build #121262 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/121262/testReport)** for PR 28195 at commit [`b1be35e`](https://github.com/apache/spark/commit/b1be35ec702f2caaf15a382a4e4aa80a32fac716).
* This patch **fails PySpark unit tests**.
* This patch merges cleanly.
* This patch adds no public classes.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA removed a comment on issue #28195:
[SPARK-31425][SQL][CORE]
UnsafeKVExternalSorter/VariableLengthRowBasedKeyValueBatch should also
respect UnsafeAlignedOffset
Posted by GitBox <gi...@apache.org>.
SparkQA removed a comment on issue #28195: [SPARK-31425][SQL][CORE] UnsafeKVExternalSorter/VariableLengthRowBasedKeyValueBatch should also respect UnsafeAlignedOffset
URL: https://github.com/apache/spark/pull/28195#issuecomment-613277335
**[Test build #121262 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/121262/testReport)** for PR 28195 at commit [`b1be35e`](https://github.com/apache/spark/commit/b1be35ec702f2caaf15a382a4e4aa80a32fac716).
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins commented on issue #28195:
[SPARK-31425][SQL][CORE]
UnsafeKVExternalSorter/VariableLengthRowBasedKeyValueBatch should also
respect UnsafeAlignedOffset
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on issue #28195: [SPARK-31425][SQL][CORE] UnsafeKVExternalSorter/VariableLengthRowBasedKeyValueBatch should also respect UnsafeAlignedOffset
URL: https://github.com/apache/spark/pull/28195#issuecomment-613532619
Test FAILed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/121276/
Test FAILed.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA removed a comment on issue #28195:
[SPARK-31425][SQL][CORE]
UnsafeKVExternalSorter/VariableLengthRowBasedKeyValueBatch should also
respect UnsafeAlignedOffset
Posted by GitBox <gi...@apache.org>.
SparkQA removed a comment on issue #28195: [SPARK-31425][SQL][CORE] UnsafeKVExternalSorter/VariableLengthRowBasedKeyValueBatch should also respect UnsafeAlignedOffset
URL: https://github.com/apache/spark/pull/28195#issuecomment-613413809
**[Test build #121276 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/121276/testReport)** for PR 28195 at commit [`bda5823`](https://github.com/apache/spark/commit/bda58233051227a679042abf77f859effbe29169).
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins removed a comment on issue #28195:
[SPARK-31425][SQL][CORE]
UnsafeKVExternalSorter/VariableLengthRowBasedKeyValueBatch should also
respect UnsafeAlignedOffset
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on issue #28195: [SPARK-31425][SQL][CORE] UnsafeKVExternalSorter/VariableLengthRowBasedKeyValueBatch should also respect UnsafeAlignedOffset
URL: https://github.com/apache/spark/pull/28195#issuecomment-613644257
Merged build finished. Test PASSed.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] cloud-fan commented on issue #28195:
[SPARK-31425][SQL][CORE]
UnsafeKVExternalSorter/VariableLengthRowBasedKeyValueBatch should also
respect UnsafeAlignedOffset
Posted by GitBox <gi...@apache.org>.
cloud-fan commented on issue #28195: [SPARK-31425][SQL][CORE] UnsafeKVExternalSorter/VariableLengthRowBasedKeyValueBatch should also respect UnsafeAlignedOffset
URL: https://github.com/apache/spark/pull/28195#issuecomment-615039499
thanks, merging to master/3.0!
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins removed a comment on issue #28195:
[SPARK-31425][SQL][CORE] UnsafeKVExternalSorter should also respect
UnsafeAlignedOffset
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on issue #28195: [SPARK-31425][SQL][CORE] UnsafeKVExternalSorter should also respect UnsafeAlignedOffset
URL: https://github.com/apache/spark/pull/28195#issuecomment-612649414
Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/121147/
Test PASSed.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins commented on issue #28195:
[SPARK-31425][SQL][CORE]
UnsafeKVExternalSorter/VariableLengthRowBasedKeyValueBatch should also
respect UnsafeAlignedOffset
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on issue #28195: [SPARK-31425][SQL][CORE] UnsafeKVExternalSorter/VariableLengthRowBasedKeyValueBatch should also respect UnsafeAlignedOffset
URL: https://github.com/apache/spark/pull/28195#issuecomment-613368584
Test FAILed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/121262/
Test FAILed.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] Ngone51 commented on issue #28195: [SPARK-31425][SQL][CORE]
UnsafeKVExternalSorter should also respect UnsafeAlignedOffset
Posted by GitBox <gi...@apache.org>.
Ngone51 commented on issue #28195: [SPARK-31425][SQL][CORE] UnsafeKVExternalSorter should also respect UnsafeAlignedOffset
URL: https://github.com/apache/spark/pull/28195#issuecomment-612833274
> can we have a test?
yet, I updated `HashAggregationQueryWithControlledFallbackSuite` to test both 4 and 8 `uaoSize`, which actually can fail with `uaoSize = 8`.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins removed a comment on issue #28195:
[SPARK-31425][SQL][CORE]
UnsafeKVExternalSorter/VariableLengthRowBasedKeyValueBatch should also
respect UnsafeAlignedOffset
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on issue #28195: [SPARK-31425][SQL][CORE] UnsafeKVExternalSorter/VariableLengthRowBasedKeyValueBatch should also respect UnsafeAlignedOffset
URL: https://github.com/apache/spark/pull/28195#issuecomment-613818640
Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/121297/
Test PASSed.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA commented on issue #28195: [SPARK-31425][SQL][CORE]
UnsafeKVExternalSorter/VariableLengthRowBasedKeyValueBatch should also
respect UnsafeAlignedOffset
Posted by GitBox <gi...@apache.org>.
SparkQA commented on issue #28195: [SPARK-31425][SQL][CORE] UnsafeKVExternalSorter/VariableLengthRowBasedKeyValueBatch should also respect UnsafeAlignedOffset
URL: https://github.com/apache/spark/pull/28195#issuecomment-614014662
**[Test build #121324 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/121324/testReport)** for PR 28195 at commit [`0fb6024`](https://github.com/apache/spark/commit/0fb6024e1b8007a295a69d4d4e99dd252a38cc9c).
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA removed a comment on issue #28195:
[SPARK-31425][SQL][CORE]
UnsafeKVExternalSorter/VariableLengthRowBasedKeyValueBatch should also
respect UnsafeAlignedOffset
Posted by GitBox <gi...@apache.org>.
SparkQA removed a comment on issue #28195: [SPARK-31425][SQL][CORE] UnsafeKVExternalSorter/VariableLengthRowBasedKeyValueBatch should also respect UnsafeAlignedOffset
URL: https://github.com/apache/spark/pull/28195#issuecomment-613773713
**[Test build #121297 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/121297/testReport)** for PR 28195 at commit [`d441e53`](https://github.com/apache/spark/commit/d441e5375859d2339334e7fd431b3c635407c516).
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] cloud-fan commented on a change in pull request #28195:
[SPARK-31425][SQL][CORE] UnsafeKVExternalSorter should also respect
UnsafeAlignedOffset
Posted by GitBox <gi...@apache.org>.
cloud-fan commented on a change in pull request #28195: [SPARK-31425][SQL][CORE] UnsafeKVExternalSorter should also respect UnsafeAlignedOffset
URL: https://github.com/apache/spark/pull/28195#discussion_r407323381
##########
File path: core/src/main/java/org/apache/spark/unsafe/map/BytesToBytesMap.java
##########
@@ -706,7 +706,7 @@ public boolean append(Object kbase, long koff, int klen, Object vbase, long voff
// Here, we'll copy the data into our data pages. Because we only store a relative offset from
// the key address instead of storing the absolute address of the value, the key and value
// must be stored in the same memory page.
- // (8 byte key length) (key) (value) (8 byte pointer to next value)
+ // (uao: klen + vlen + uaolen) (uao: klen) (key) (value) (8 byte pointer to next value)
Review comment:
it's `(total length) (key length) ...`?
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins removed a comment on issue #28195:
[SPARK-31425][SQL][CORE]
UnsafeKVExternalSorter/VariableLengthRowBasedKeyValueBatch should also
respect UnsafeAlignedOffset
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on issue #28195: [SPARK-31425][SQL][CORE] UnsafeKVExternalSorter/VariableLengthRowBasedKeyValueBatch should also respect UnsafeAlignedOffset
URL: https://github.com/apache/spark/pull/28195#issuecomment-613414292
Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder-K8s/25963/
Test PASSed.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins removed a comment on issue #28195:
[SPARK-31425][SQL][CORE] UnsafeKVExternalSorter should also respect
UnsafeAlignedOffset
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on issue #28195: [SPARK-31425][SQL][CORE] UnsafeKVExternalSorter should also respect UnsafeAlignedOffset
URL: https://github.com/apache/spark/pull/28195#issuecomment-613277934
Merged build finished. Test PASSed.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org