You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@orc.apache.org by "wankunde (via GitHub)" <gi...@apache.org> on 2025/09/03 10:26:26 UTC
[PR] ORC-1986: Trigger flush stripe for large input rows [orc]
wankunde opened a new pull request, #2371:
URL: https://github.com/apache/orc/pull/2371
### What changes were proposed in this pull request?
For large input rows, the stripe may excessively large , requiring more memory for both reading and writing one strip. We can check the tree write size in bytes and flush the strip even when the input rows count is less than 5000.
```log
Stripes:
Stripe: offset: 3 data: 347494188 rows: 5120 tail: 244 index: 2304
Stream: column 0 section ROW_INDEX start: 3 length 12
Stream: column 1 section ROW_INDEX start: 15 length 110
Stream: column 2 section ROW_INDEX start: 125 length 893
Stream: column 3 section ROW_INDEX start: 1018 length 31
Stream: column 4 section ROW_INDEX start: 1049 length 65
Stream: column 5 section ROW_INDEX start: 1114 length 923
Stream: column 6 section ROW_INDEX start: 2037 length 25
Stream: column 7 section ROW_INDEX start: 2062 length 155
Stream: column 8 section ROW_INDEX start: 2217 length 28
Stream: column 9 section ROW_INDEX start: 2245 length 31
Stream: column 10 section ROW_INDEX start: 2276 length 31
Stream: column 1 section DATA start: 2307 length 81853
Stream: column 1 section LENGTH start: 84160 length 2191
Stream: column 2 section DATA start: 86351 length 345862763
Stream: column 2 section LENGTH start: 345949114 length 13736
Stream: column 3 section DATA start: 345962850 length 22
Stream: column 3 section LENGTH start: 345962872 length 6
Stream: column 3 section DICTIONARY_DATA start: 345962878 length 5
Stream: column 4 section PRESENT start: 345962883 length 200
Stream: column 4 section DATA start: 345963083 length 6322
Stream: column 4 section LENGTH start: 345969405 length 495
Stream: column 4 section DICTIONARY_DATA start: 345969900 length 2919
Stream: column 5 section DATA start: 345972819 length 1507883
Stream: column 5 section LENGTH start: 347480702 length 7346
Stream: column 6 section DATA start: 347488048 length 22
Stream: column 6 section LENGTH start: 347488070 length 6
Stream: column 6 section DICTIONARY_DATA start: 347488076 length 0
Stream: column 7 section DATA start: 347488076 length 5795
Stream: column 7 section LENGTH start: 347493871 length 301
Stream: column 7 section DICTIONARY_DATA start: 347494172 length 2187
Stream: column 8 section DATA start: 347496359 length 22
Stream: column 8 section LENGTH start: 347496381 length 6
Stream: column 8 section DICTIONARY_DATA start: 347496387 length 4
Stream: column 9 section DATA start: 347496391 length 58
Stream: column 9 section LENGTH start: 347496449 length 6
Stream: column 9 section DICTIONARY_DATA start: 347496455 length 7
Stream: column 10 section DATA start: 347496462 length 22
Stream: column 10 section LENGTH start: 347496484 length 6
Stream: column 10 section DICTIONARY_DATA start: 347496490 length 5
Encoding column 0: DIRECT
Encoding column 1: DIRECT_V2
Encoding column 2: DIRECT_V2
Encoding column 3: DICTIONARY_V2[1]
Encoding column 4: DICTIONARY_V2[661]
Encoding column 5: DIRECT_V2
Encoding column 6: DICTIONARY_V2[1]
Encoding column 7: DICTIONARY_V2[682]
Encoding column 8: DICTIONARY_V2[1]
Encoding column 9: DICTIONARY_V2[2]
Encoding column 10: DICTIONARY_V2[1]
```
### Why are the changes needed?
To optimize the memory usage.
### How was this patch tested?
Local test
### Was this patch authored or co-authored using generative AI tooling?
No
--
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.
To unsubscribe, e-mail: issues-unsubscribe@orc.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] ORC-1986: Trigger flush stripe for large input rows [orc]
Posted by "cxzl25 (via GitHub)" <gi...@apache.org>.
cxzl25 commented on code in PR #2371:
URL: https://github.com/apache/orc/pull/2371#discussion_r2371835867
##########
java/core/src/java/org/apache/orc/impl/WriterImpl.java:
##########
@@ -224,6 +225,8 @@ public WriterImpl(FileSystem fs,
this.stripeRowCount= opts.getStripeRowCountValue();
this.stripeSize = opts.getStripeSize();
memoryLimit = stripeSize;
+ double stripeSizeCheckRatio = OrcConf.STRIPE_SIZE_CHECKRATIO.getDouble(conf);
+ STRIPE_SIZE_PER_CHECK = stripeSizeCheckRatio <= 0 ? 0 : stripeSizeCheckRatio * memoryLimit;
Review Comment:
```suggestion
STRIPE_SIZE_PER_CHECK = stripeSizeCheckRatio <= 0 ? 0 : stripeSizeCheckRatio * stripeSize;
```
--
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.
To unsubscribe, e-mail: issues-unsubscribe@orc.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] ORC-1986: Trigger flush stripe for large input rows [orc]
Posted by "wankunde (via GitHub)" <gi...@apache.org>.
wankunde commented on PR #2371:
URL: https://github.com/apache/orc/pull/2371#issuecomment-3258345388
Hi, @cxzl25 @dongjoon-hyun
Add local benchmark result, orc writer needs less compute time and memory than before.
<img width="622" height="234" alt="企业微信截图_c9487097-1e43-4890-8dad-243399915d01" src="https://github.com/user-attachments/assets/42dc2c70-8286-4840-835d-8aefb5fd8dab" />
```log
Before:
Batch Size: 1024, column Num: 10, took 4 ms to write 10000 rows, flush stripe time: 0 ms, flush stripe count: 0
Batch Size: 1024, column Num: 100, took 43 ms to write 10000 rows, flush stripe time: 0 ms, flush stripe count: 0
Batch Size: 1024, column Num: 300, took 80 ms to write 10000 rows, flush stripe time: 0 ms, flush stripe count: 0
Batch Size: 1024, column Num: 500, took 111 ms to write 10000 rows, flush stripe time: 0 ms, flush stripe count: 0
Batch Size: 1024, column Num: 1000, took 234 ms to write 10000 rows, flush stripe time: 0 ms, flush stripe count: 0
Batch Size: 1024, column Num: 5000, took 1884 ms to write 10000 rows, flush stripe time: 632 ms, flush stripe count: 1
Batch Size: 1024, column Num: 10000, took 5132 ms to write 10000 rows, flush stripe time: 2391 ms, flush stripe count: 1
After:
Batch Size: 1024, column Num: 10, took 3 ms to write 10000 rows, flush stripe time: 0 ms, flush stripe count: 0
Batch Size: 1024, column Num: 100, took 33 ms to write 10000 rows, flush stripe time: 0 ms, flush stripe count: 0
Batch Size: 1024, column Num: 300, took 71 ms to write 10000 rows, flush stripe time: 0 ms, flush stripe count: 0
Batch Size: 1024, column Num: 500, took 112 ms to write 10000 rows, flush stripe time: 0 ms, flush stripe count: 0
Batch Size: 1024, column Num: 1000, took 233 ms to write 10000 rows, flush stripe time: 0 ms, flush stripe count: 0
Batch Size: 1024, column Num: 5000, took 5984 ms to write 10000 rows, flush stripe time: 5100 ms, flush stripe count: 10
Batch Size: 1024, column Num: 10000, took 20473 ms to write 10000 rows, flush stripe time: 19308 ms, flush stripe count: 10
```
--
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.
To unsubscribe, e-mail: issues-unsubscribe@orc.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] ORC-1986: Trigger flush stripe for large input rows [orc]
Posted by "dongjoon-hyun (via GitHub)" <gi...@apache.org>.
dongjoon-hyun commented on code in PR #2371:
URL: https://github.com/apache/orc/pull/2371#discussion_r2360996684
##########
java/tools/src/test/org/apache/orc/tools/TestFileDump.java:
##########
@@ -83,6 +83,7 @@ public class TestFileDump implements TestConf {
@BeforeEach
public void openFileSystem () throws Exception {
+ conf.set("orc.stripe.size.checkRatio", "0");
Review Comment:
ditto. Why do we disable new feature in this test file?
--
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.
To unsubscribe, e-mail: issues-unsubscribe@orc.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] ORC-1986: Trigger flush stripe for large input rows [orc]
Posted by "dongjoon-hyun (via GitHub)" <gi...@apache.org>.
dongjoon-hyun closed pull request #2371: ORC-1986: Trigger flush stripe for large input rows
URL: https://github.com/apache/orc/pull/2371
--
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.
To unsubscribe, e-mail: issues-unsubscribe@orc.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] ORC-1986: Trigger flush stripe for large input rows [orc]
Posted by "dongjoon-hyun (via GitHub)" <gi...@apache.org>.
dongjoon-hyun commented on code in PR #2371:
URL: https://github.com/apache/orc/pull/2371#discussion_r2360954906
##########
java/core/src/java/org/apache/orc/OrcConf.java:
##########
@@ -182,6 +187,11 @@ public enum OrcConf {
"added to all of the writers. Valid range is [1,10000] and is primarily meant for" +
"testing. Setting this too low may negatively affect performance."
+ " Use orc.stripe.row.count instead if the value larger than orc.stripe.row.count."),
+ STRIPE_SIZE_CHECKRATIO("orc.stripe.size.checkRatio",
+ "hive.exec.orc.default.stripe.size.checkRatio",
Review Comment:
Ditto. Let's not introduce `Apache Hive` configuration.
--
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.
To unsubscribe, e-mail: issues-unsubscribe@orc.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] ORC-1986: Trigger flush stripe for large input rows [orc]
Posted by "wankunde (via GitHub)" <gi...@apache.org>.
wankunde commented on PR #2371:
URL: https://github.com/apache/orc/pull/2371#issuecomment-3290280501
@cxzl25 Thanks for your review. Change `STRIPE_SIZE_CHECK` to `OrcConf.STRIPE_SIZE_CHECKRATIO * stripeSize`
--
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.
To unsubscribe, e-mail: issues-unsubscribe@orc.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] ORC-1986: Trigger flush stripe for large input rows [orc]
Posted by "dongjoon-hyun (via GitHub)" <gi...@apache.org>.
dongjoon-hyun commented on code in PR #2371:
URL: https://github.com/apache/orc/pull/2371#discussion_r2360995697
##########
java/core/src/test/org/apache/orc/TestVectorOrcFile.java:
##########
@@ -197,6 +197,7 @@ private static ByteBuffer byteBuf(int... items) {
@BeforeEach
public void openFileSystem(TestInfo testInfo) throws Exception {
+ conf.set("orc.stripe.size.checkRatio", "0");
Review Comment:
May I ask why we disable new feature in this test file?
--
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.
To unsubscribe, e-mail: issues-unsubscribe@orc.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] ORC-1986: Trigger flush stripe for large input rows [orc]
Posted by "dongjoon-hyun (via GitHub)" <gi...@apache.org>.
dongjoon-hyun commented on code in PR #2371:
URL: https://github.com/apache/orc/pull/2371#discussion_r2360978006
##########
java/core/src/java/org/apache/orc/impl/WriterImpl.java:
##########
@@ -112,6 +112,7 @@ public class WriterImpl implements WriterInternal, MemoryManager.Callback {
private long previousAllocation = -1;
private long memoryLimit;
private final long ROWS_PER_CHECK;
+ private final double STRIPE_SIZE_CHECK;
Review Comment:
To be consistent with `ROWS_PER_CHECK`, `STRIPE_SIZE_CHECK` -> `STRIPE_SIZE_PER_CHECK`.
--
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.
To unsubscribe, e-mail: issues-unsubscribe@orc.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] ORC-1986: Trigger flush stripe for large input rows [orc]
Posted by "wankunde (via GitHub)" <gi...@apache.org>.
wankunde commented on code in PR #2371:
URL: https://github.com/apache/orc/pull/2371#discussion_r2324590406
##########
java/core/src/java/org/apache/orc/OrcConf.java:
##########
@@ -182,6 +187,9 @@ public enum OrcConf {
"added to all of the writers. Valid range is [1,10000] and is primarily meant for" +
"testing. Setting this too low may negatively affect performance."
+ " Use orc.stripe.row.count instead if the value larger than orc.stripe.row.count."),
+ STRIPE_SIZE_CHECK("orc.stripe.size.check", "hive.exec.orc.default.stripe.size.check",
+ 128L * 1024 * 1024,
Review Comment:
The default stripe size is 64MB, so I think 128MB is large enough to flush this strip.
```
STRIPE_SIZE("orc.stripe.size", "hive.exec.orc.default.stripe.size",
64L * 1024 * 1024,
"Define the default ORC stripe size, in bytes."),
```
Test with our production jobs, our spark jobs could run with 6GB executors if STRIPE_SIZE_CHECK = 128MB, and need 8GB executors if STRIPE_SIZE_CHECK = 256MB
--
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.
To unsubscribe, e-mail: issues-unsubscribe@orc.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] ORC-1986: Trigger flush stripe for large input rows [orc]
Posted by "wankunde (via GitHub)" <gi...@apache.org>.
wankunde commented on code in PR #2371:
URL: https://github.com/apache/orc/pull/2371#discussion_r2320727675
##########
java/core/src/java/org/apache/orc/OrcConf.java:
##########
@@ -118,6 +118,11 @@ public enum OrcConf {
"If the number of distinct keys in a dictionary is greater than this\n" +
"fraction of the total number of non-null rows, turn off \n" +
"dictionary encoding. Use 1 to always use dictionary encoding."),
+ DICTIONARY_MAX_SIZE_IN_BYTES("orc.dictionary.maxSizeInBytes",
+ "hive.exec.orc.dictionary.maxSizeInBytes",
+ 16 * 1024 * 1024,
+ "If the total size of the dictionary is greater than this\n" +
+ ", turn off dictionary encoding. Use 1 to always use dictionary encoding."),
Review Comment:
Remove this unused comment, thanks for your review
--
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.
To unsubscribe, e-mail: issues-unsubscribe@orc.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] ORC-1986: Trigger flush stripe for large input rows [orc]
Posted by "wankunde (via GitHub)" <gi...@apache.org>.
wankunde commented on PR #2371:
URL: https://github.com/apache/orc/pull/2371#issuecomment-3294612944
Hi, @dongjoon-hyun do you have any thoughts on this PR ?
--
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.
To unsubscribe, e-mail: issues-unsubscribe@orc.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] ORC-1986: Trigger flush stripe for large input rows [orc]
Posted by "cxzl25 (via GitHub)" <gi...@apache.org>.
cxzl25 commented on code in PR #2371:
URL: https://github.com/apache/orc/pull/2371#discussion_r2320677366
##########
java/core/src/java/org/apache/orc/OrcConf.java:
##########
@@ -118,6 +118,11 @@ public enum OrcConf {
"If the number of distinct keys in a dictionary is greater than this\n" +
"fraction of the total number of non-null rows, turn off \n" +
"dictionary encoding. Use 1 to always use dictionary encoding."),
+ DICTIONARY_MAX_SIZE_IN_BYTES("orc.dictionary.maxSizeInBytes",
+ "hive.exec.orc.dictionary.maxSizeInBytes",
+ 16 * 1024 * 1024,
+ "If the total size of the dictionary is greater than this\n" +
+ ", turn off dictionary encoding. Use 1 to always use dictionary encoding."),
Review Comment:
> Use 1 to always use dictionary encoding.
It seems that this logic is not implemented.
--
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.
To unsubscribe, e-mail: issues-unsubscribe@orc.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] ORC-1986: Trigger flush stripe for large input rows [orc]
Posted by "wankunde (via GitHub)" <gi...@apache.org>.
wankunde commented on code in PR #2371:
URL: https://github.com/apache/orc/pull/2371#discussion_r2324577133
##########
java/core/src/java/org/apache/orc/OrcConf.java:
##########
@@ -118,6 +118,11 @@ public enum OrcConf {
"If the number of distinct keys in a dictionary is greater than this\n" +
"fraction of the total number of non-null rows, turn off \n" +
"dictionary encoding. Use 1 to always use dictionary encoding."),
+ DICTIONARY_MAX_SIZE_IN_BYTES("orc.dictionary.maxSizeInBytes",
+ "hive.exec.orc.dictionary.maxSizeInBytes",
+ 16 * 1024 * 1024,
Review Comment:
```log
dictionary.size = 5120, row size = 5120, dictionary SizeInBytes = 360448, rows SizeInBytes = 32768
dictionary.size = 5120, row size = 5120, dictionary SizeInBytes = 1452277760, rows SizeInBytes = 32768
dictionary.size = 1, row size = 5120, dictionary SizeInBytes = 81920, rows SizeInBytes = 32768
dictionary.size = 661, row size = 5012, dictionary SizeInBytes = 81920, rows SizeInBytes = 32768
dictionary.size = 5120, row size = 5120, dictionary SizeInBytes = 10223616, rows SizeInBytes = 32768
dictionary.size = 1, row size = 5120, dictionary SizeInBytes = 81920, rows SizeInBytes = 32768
dictionary.size = 682, row size = 5120, dictionary SizeInBytes = 147456, rows SizeInBytes = 32768
dictionary.size = 1, row size = 5120, dictionary SizeInBytes = 81920, rows SizeInBytes = 32768
dictionary.size = 2, row size = 5120, dictionary SizeInBytes = 81920, rows SizeInBytes = 32768
dictionary.size = 1, row size = 5120, dictionary SizeInBytes = 81920, rows SizeInBytes = 32768
```
The dictionary size in bytes usually less than 10MB.
If dictionary size (>16MB) / stripe size(< 128MB) > 12.5%, I think it large enough to disable the dictionary encoding.
--
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.
To unsubscribe, e-mail: issues-unsubscribe@orc.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] ORC-1986: Trigger flush stripe for large input rows [orc]
Posted by "cxzl25 (via GitHub)" <gi...@apache.org>.
cxzl25 commented on code in PR #2371:
URL: https://github.com/apache/orc/pull/2371#discussion_r2344221223
##########
java/core/src/java/org/apache/orc/OrcConf.java:
##########
@@ -182,6 +187,9 @@ public enum OrcConf {
"added to all of the writers. Valid range is [1,10000] and is primarily meant for" +
"testing. Setting this too low may negatively affect performance."
+ " Use orc.stripe.row.count instead if the value larger than orc.stripe.row.count."),
+ STRIPE_SIZE_CHECK("orc.stripe.size.check", "hive.exec.orc.default.stripe.size.check",
+ 128L * 1024 * 1024,
Review Comment:
Can you consider the configuration according to the scale of stripe size?
`orc.stripe.size`*ratio
##########
java/core/src/java/org/apache/orc/impl/WriterImpl.java:
##########
@@ -325,9 +327,9 @@ public boolean checkMemory(double newScale) throws IOException {
}
private boolean checkMemory() throws IOException {
- if (rowsSinceCheck >= ROWS_PER_CHECK) {
+ long size = treeWriter.estimateMemory();
Review Comment:
I suggest that if the conditions do not meet the requirements, the call frequency of `estimateMemory` is appropriately reduced.
```suggestion
long size = rowsSinceCheck < ROWS_PER_CHECK && STRIPE_SIZE_CHECK <= 0
? 0 : treeWriter.estimateMemory();
```
--
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.
To unsubscribe, e-mail: issues-unsubscribe@orc.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] ORC-1986: Trigger flush stripe for large input rows [orc]
Posted by "wankunde (via GitHub)" <gi...@apache.org>.
wankunde commented on code in PR #2371:
URL: https://github.com/apache/orc/pull/2371#discussion_r2361911982
##########
java/core/src/java/org/apache/orc/OrcConf.java:
##########
@@ -118,6 +118,11 @@ public enum OrcConf {
"If the number of distinct keys in a dictionary is greater than this\n" +
"fraction of the total number of non-null rows, turn off \n" +
"dictionary encoding. Use 1 to always use dictionary encoding."),
+ DICTIONARY_MAX_SIZE_IN_BYTES("orc.dictionary.maxSizeInBytes",
+ "hive.exec.orc.dictionary.maxSizeInBytes",
Review Comment:
Done
##########
java/core/src/java/org/apache/orc/OrcConf.java:
##########
@@ -182,6 +187,11 @@ public enum OrcConf {
"added to all of the writers. Valid range is [1,10000] and is primarily meant for" +
"testing. Setting this too low may negatively affect performance."
+ " Use orc.stripe.row.count instead if the value larger than orc.stripe.row.count."),
+ STRIPE_SIZE_CHECKRATIO("orc.stripe.size.checkRatio",
+ "hive.exec.orc.default.stripe.size.checkRatio",
Review Comment:
Done
##########
java/core/src/java/org/apache/orc/OrcConf.java:
##########
@@ -182,6 +187,11 @@ public enum OrcConf {
"added to all of the writers. Valid range is [1,10000] and is primarily meant for" +
"testing. Setting this too low may negatively affect performance."
+ " Use orc.stripe.row.count instead if the value larger than orc.stripe.row.count."),
+ STRIPE_SIZE_CHECKRATIO("orc.stripe.size.checkRatio",
+ "hive.exec.orc.default.stripe.size.checkRatio",
+ 2.0,
+ "Flush stripe if the tree writer size in bytes is larger than (this * orc.stripe.size), " +
Review Comment:
Done
--
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.
To unsubscribe, e-mail: issues-unsubscribe@orc.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] ORC-1986: Trigger flush stripe for large input rows [orc]
Posted by "wankunde (via GitHub)" <gi...@apache.org>.
wankunde commented on PR #2371:
URL: https://github.com/apache/orc/pull/2371#issuecomment-3264494344
Hi, @cxzl25 @dongjoon-hyun
* Allow use 0 to disable this new check.
* Add local benchmark, https://github.com/apache/orc/commit/733834d787176e9641979bbc7eff6b3f1e557984
<img width="1135" height="445" alt="image" src="https://github.com/user-attachments/assets/4d9e1bad-4b63-4d2a-a245-f7b4c6d09678" />
--
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.
To unsubscribe, e-mail: issues-unsubscribe@orc.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] ORC-1986: Trigger flush stripe for large input rows [orc]
Posted by "dongjoon-hyun (via GitHub)" <gi...@apache.org>.
dongjoon-hyun commented on code in PR #2371:
URL: https://github.com/apache/orc/pull/2371#discussion_r2360965245
##########
java/core/src/java/org/apache/orc/OrcConf.java:
##########
@@ -182,6 +187,11 @@ public enum OrcConf {
"added to all of the writers. Valid range is [1,10000] and is primarily meant for" +
"testing. Setting this too low may negatively affect performance."
+ " Use orc.stripe.row.count instead if the value larger than orc.stripe.row.count."),
+ STRIPE_SIZE_CHECKRATIO("orc.stripe.size.checkRatio",
+ "hive.exec.orc.default.stripe.size.checkRatio",
+ 2.0,
+ "Flush stripe if the tree writer size in bytes is larger than (this * orc.stripe.size), " +
Review Comment:
`, ` -> `. `
--
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.
To unsubscribe, e-mail: issues-unsubscribe@orc.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] ORC-1986: Trigger flush stripe for large input rows [orc]
Posted by "wankunde (via GitHub)" <gi...@apache.org>.
wankunde commented on code in PR #2371:
URL: https://github.com/apache/orc/pull/2371#discussion_r2361912574
##########
java/core/src/java/org/apache/orc/OrcConf.java:
##########
@@ -182,6 +187,11 @@ public enum OrcConf {
"added to all of the writers. Valid range is [1,10000] and is primarily meant for" +
"testing. Setting this too low may negatively affect performance."
+ " Use orc.stripe.row.count instead if the value larger than orc.stripe.row.count."),
+ STRIPE_SIZE_CHECKRATIO("orc.stripe.size.checkRatio",
+ "hive.exec.orc.default.stripe.size.checkRatio",
+ 2.0,
+ "Flush stripe if the tree writer size in bytes is larger than (this * orc.stripe.size), " +
+ "use 0 to disable this check."),
Review Comment:
Done
##########
java/core/src/java/org/apache/orc/impl/WriterImpl.java:
##########
@@ -112,6 +112,7 @@ public class WriterImpl implements WriterInternal, MemoryManager.Callback {
private long previousAllocation = -1;
private long memoryLimit;
private final long ROWS_PER_CHECK;
+ private final double STRIPE_SIZE_CHECK;
Review Comment:
Done
--
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.
To unsubscribe, e-mail: issues-unsubscribe@orc.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] ORC-1986: Trigger flush stripe for large input rows [orc]
Posted by "wankunde (via GitHub)" <gi...@apache.org>.
wankunde commented on code in PR #2371:
URL: https://github.com/apache/orc/pull/2371#discussion_r2361942741
##########
java/tools/src/test/org/apache/orc/tools/TestFileDump.java:
##########
@@ -83,6 +83,7 @@ public class TestFileDump implements TestConf {
@BeforeEach
public void openFileSystem () throws Exception {
+ conf.set("orc.stripe.size.checkRatio", "0");
Review Comment:
The same reason with `TestVectorOrcFile`
--
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.
To unsubscribe, e-mail: issues-unsubscribe@orc.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] ORC-1986: Trigger flush stripe for large input rows [orc]
Posted by "wankunde (via GitHub)" <gi...@apache.org>.
wankunde commented on PR #2371:
URL: https://github.com/apache/orc/pull/2371#issuecomment-3248950158
Hi, @dongjoon-hyun could you help to review this PR ? 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.
To unsubscribe, e-mail: issues-unsubscribe@orc.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] ORC-1986: Trigger flush stripe for large input rows [orc]
Posted by "dongjoon-hyun (via GitHub)" <gi...@apache.org>.
dongjoon-hyun commented on code in PR #2371:
URL: https://github.com/apache/orc/pull/2371#discussion_r2323137620
##########
java/core/src/java/org/apache/orc/OrcConf.java:
##########
@@ -118,6 +118,11 @@ public enum OrcConf {
"If the number of distinct keys in a dictionary is greater than this\n" +
"fraction of the total number of non-null rows, turn off \n" +
"dictionary encoding. Use 1 to always use dictionary encoding."),
+ DICTIONARY_MAX_SIZE_IN_BYTES("orc.dictionary.maxSizeInBytes",
+ "hive.exec.orc.dictionary.maxSizeInBytes",
+ 16 * 1024 * 1024,
Review Comment:
May I ask why this value is the default, @wankunde ?
--
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.
To unsubscribe, e-mail: issues-unsubscribe@orc.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] ORC-1986: Trigger flush stripe for large input rows [orc]
Posted by "cxzl25 (via GitHub)" <gi...@apache.org>.
cxzl25 commented on code in PR #2371:
URL: https://github.com/apache/orc/pull/2371#discussion_r2320761940
##########
java/core/src/java/org/apache/orc/impl/WriterImpl.java:
##########
@@ -325,9 +327,9 @@ public boolean checkMemory(double newScale) throws IOException {
}
private boolean checkMemory() throws IOException {
- if (rowsSinceCheck >= ROWS_PER_CHECK) {
+ long size = treeWriter.estimateMemory();
Review Comment:
If `estimateMemory` is called for each batch write, will the performance of the write be degraded?
--
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.
To unsubscribe, e-mail: issues-unsubscribe@orc.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] ORC-1986: Trigger flush stripe for large input rows [orc]
Posted by "wankunde (via GitHub)" <gi...@apache.org>.
wankunde commented on code in PR #2371:
URL: https://github.com/apache/orc/pull/2371#discussion_r2321162659
##########
java/core/src/java/org/apache/orc/impl/WriterImpl.java:
##########
@@ -325,9 +327,9 @@ public boolean checkMemory(double newScale) throws IOException {
}
private boolean checkMemory() throws IOException {
- if (rowsSinceCheck >= ROWS_PER_CHECK) {
+ long size = treeWriter.estimateMemory();
Review Comment:
* If rowsSinceCheck >= ROWS_PER_CHECK, the `estimateMemory` will be called the same as before
* If rowsSinceCheck < ROWS_PER_CHECK, the `estimateMemory` will be called `ROWS_PER_CHECK / VectorizedRowBatch.size` times, and `5000 / 1024 = 4` times by default, I think this is much smaller overhead in the overall 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.
To unsubscribe, e-mail: issues-unsubscribe@orc.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] ORC-1986: Trigger flush stripe for large input rows [orc]
Posted by "wankunde (via GitHub)" <gi...@apache.org>.
wankunde commented on code in PR #2371:
URL: https://github.com/apache/orc/pull/2371#discussion_r2361940505
##########
java/core/src/test/org/apache/orc/TestVectorOrcFile.java:
##########
@@ -197,6 +197,7 @@ private static ByteBuffer byteBuf(int... items) {
@BeforeEach
public void openFileSystem(TestInfo testInfo) throws Exception {
+ conf.set("orc.stripe.size.checkRatio", "0");
Review Comment:
Because in these two UT files, some orc writers initialize with `.stripeSize(100000)` and write more stripes than before.
See https://github.com/apache/orc/actions/runs/17724428261/job/50362336226
```java
Error: Tests run: 10, Failures: 2, Errors: 0, Skipped: 0, Time elapsed: 1.282 s <<< FAILURE! -- in org.apache.orc.tools.TestFileDump
Error: org.apache.orc.tools.TestFileDump.testDictionaryThreshold -- Time elapsed: 0.176 s <<< FAILURE!
org.opentest4j.AssertionFailedError: expected: <Column 0: count: 5000 hasNull: false> but was: <Column 0: count: 3000 hasNull: false>
at org.junit.jupiter.api.AssertionFailureBuilder.build(AssertionFailureBuilder.java:151)
at org.junit.jupiter.api.AssertionFailureBuilder.buildAndThrow(AssertionFailureBuilder.java:132)
at org.junit.jupiter.api.AssertEquals.failNotEqual(AssertEquals.java:197)
at org.junit.jupiter.api.AssertEquals.assertEquals(AssertEquals.java:182)
at org.junit.jupiter.api.AssertEquals.assertEquals(AssertEquals.java:177)
at org.junit.jupiter.api.Assertions.assertEquals(Assertions.java:1145)
at org.apache.orc.tools.TestFileDump.checkOutput(TestFileDump.java:243)
at org.apache.orc.tools.TestFileDump.testDictionaryThreshold(TestFileDump.java:443)
at java.base/java.lang.reflect.Method.invoke(Method.java:565)
at java.base/java.util.ArrayList.forEach(ArrayList.java:1604)
at java.base/java.util.ArrayList.forEach(ArrayList.java:1604)
Error: org.apache.orc.tools.TestFileDump.testHasNull -- Time elapsed: 0.029 s <<< FAILURE!
org.opentest4j.AssertionFailedError: expected: <true> but was: <false>
at org.junit.jupiter.api.AssertionFailureBuilder.build(AssertionFailureBuilder.java:151)
at org.junit.jupiter.api.AssertionFailureBuilder.buildAndThrow(AssertionFailureBuilder.java:132)
at org.junit.jupiter.api.AssertTrue.failNotTrue(AssertTrue.java:63)
at org.junit.jupiter.api.AssertTrue.assertTrue(AssertTrue.java:36)
at org.junit.jupiter.api.AssertTrue.assertTrue(AssertTrue.java:31)
at org.junit.jupiter.api.Assertions.assertTrue(Assertions.java:183)
at org.apache.orc.tools.TestFileDump.testHasNull(TestFileDump.java:669)
at java.base/java.lang.reflect.Method.invoke(Method.java:565)
at java.base/java.util.ArrayList.forEach(ArrayList.java:1604)
at java.base/java.util.ArrayList.forEach(ArrayList.java:1604)
```
--
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.
To unsubscribe, e-mail: issues-unsubscribe@orc.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] ORC-1986: Trigger flush stripe for large input rows [orc]
Posted by "dongjoon-hyun (via GitHub)" <gi...@apache.org>.
dongjoon-hyun commented on code in PR #2371:
URL: https://github.com/apache/orc/pull/2371#discussion_r2360953019
##########
java/core/src/java/org/apache/orc/OrcConf.java:
##########
@@ -118,6 +118,11 @@ public enum OrcConf {
"If the number of distinct keys in a dictionary is greater than this\n" +
"fraction of the total number of non-null rows, turn off \n" +
"dictionary encoding. Use 1 to always use dictionary encoding."),
+ DICTIONARY_MAX_SIZE_IN_BYTES("orc.dictionary.maxSizeInBytes",
+ "hive.exec.orc.dictionary.maxSizeInBytes",
Review Comment:
Let's not introduce this new `Apache Hive` configuration, `hive.exec.orc.dictionary.maxSizeInBytes`. You can simply repeat `orc.dictionary.maxSizeInBytes` here like the other configuration example.
https://github.com/apache/orc/blob/b9035b426070b1a90a6a1b2f02063d34c0eb7353/java/core/src/java/org/apache/orc/OrcConf.java#L98
--
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.
To unsubscribe, e-mail: issues-unsubscribe@orc.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] ORC-1986: Trigger flush stripe for large input rows [orc]
Posted by "dongjoon-hyun (via GitHub)" <gi...@apache.org>.
dongjoon-hyun commented on code in PR #2371:
URL: https://github.com/apache/orc/pull/2371#discussion_r2360967314
##########
java/core/src/java/org/apache/orc/OrcConf.java:
##########
@@ -182,6 +187,11 @@ public enum OrcConf {
"added to all of the writers. Valid range is [1,10000] and is primarily meant for" +
"testing. Setting this too low may negatively affect performance."
+ " Use orc.stripe.row.count instead if the value larger than orc.stripe.row.count."),
+ STRIPE_SIZE_CHECKRATIO("orc.stripe.size.checkRatio",
+ "hive.exec.orc.default.stripe.size.checkRatio",
+ 2.0,
+ "Flush stripe if the tree writer size in bytes is larger than (this * orc.stripe.size), " +
+ "use 0 to disable this check."),
Review Comment:
`use 0` -> `Use 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.
To unsubscribe, e-mail: issues-unsubscribe@orc.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] ORC-1986: Trigger flush stripe for large input rows [orc]
Posted by "wankunde (via GitHub)" <gi...@apache.org>.
wankunde commented on code in PR #2371:
URL: https://github.com/apache/orc/pull/2371#discussion_r2361940505
##########
java/core/src/test/org/apache/orc/TestVectorOrcFile.java:
##########
@@ -197,6 +197,7 @@ private static ByteBuffer byteBuf(int... items) {
@BeforeEach
public void openFileSystem(TestInfo testInfo) throws Exception {
+ conf.set("orc.stripe.size.checkRatio", "0");
Review Comment:
Because in these two UT files, some orc writers initialize with `.stripeSize(100000)` and generate more stripes than before.
See https://github.com/apache/orc/actions/runs/17724428261/job/50362336226
```java
Error: Tests run: 10, Failures: 2, Errors: 0, Skipped: 0, Time elapsed: 1.282 s <<< FAILURE! -- in org.apache.orc.tools.TestFileDump
Error: org.apache.orc.tools.TestFileDump.testDictionaryThreshold -- Time elapsed: 0.176 s <<< FAILURE!
org.opentest4j.AssertionFailedError: expected: <Column 0: count: 5000 hasNull: false> but was: <Column 0: count: 3000 hasNull: false>
at org.junit.jupiter.api.AssertionFailureBuilder.build(AssertionFailureBuilder.java:151)
at org.junit.jupiter.api.AssertionFailureBuilder.buildAndThrow(AssertionFailureBuilder.java:132)
at org.junit.jupiter.api.AssertEquals.failNotEqual(AssertEquals.java:197)
at org.junit.jupiter.api.AssertEquals.assertEquals(AssertEquals.java:182)
at org.junit.jupiter.api.AssertEquals.assertEquals(AssertEquals.java:177)
at org.junit.jupiter.api.Assertions.assertEquals(Assertions.java:1145)
at org.apache.orc.tools.TestFileDump.checkOutput(TestFileDump.java:243)
at org.apache.orc.tools.TestFileDump.testDictionaryThreshold(TestFileDump.java:443)
at java.base/java.lang.reflect.Method.invoke(Method.java:565)
at java.base/java.util.ArrayList.forEach(ArrayList.java:1604)
at java.base/java.util.ArrayList.forEach(ArrayList.java:1604)
Error: org.apache.orc.tools.TestFileDump.testHasNull -- Time elapsed: 0.029 s <<< FAILURE!
org.opentest4j.AssertionFailedError: expected: <true> but was: <false>
at org.junit.jupiter.api.AssertionFailureBuilder.build(AssertionFailureBuilder.java:151)
at org.junit.jupiter.api.AssertionFailureBuilder.buildAndThrow(AssertionFailureBuilder.java:132)
at org.junit.jupiter.api.AssertTrue.failNotTrue(AssertTrue.java:63)
at org.junit.jupiter.api.AssertTrue.assertTrue(AssertTrue.java:36)
at org.junit.jupiter.api.AssertTrue.assertTrue(AssertTrue.java:31)
at org.junit.jupiter.api.Assertions.assertTrue(Assertions.java:183)
at org.apache.orc.tools.TestFileDump.testHasNull(TestFileDump.java:669)
at java.base/java.lang.reflect.Method.invoke(Method.java:565)
at java.base/java.util.ArrayList.forEach(ArrayList.java:1604)
at java.base/java.util.ArrayList.forEach(ArrayList.java:1604)
```
--
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.
To unsubscribe, e-mail: issues-unsubscribe@orc.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] ORC-1986: Trigger flush stripe for large input rows [orc]
Posted by "cxzl25 (via GitHub)" <gi...@apache.org>.
cxzl25 commented on code in PR #2371:
URL: https://github.com/apache/orc/pull/2371#discussion_r2348926692
##########
java/core/src/java/org/apache/orc/OrcConf.java:
##########
@@ -118,6 +118,11 @@ public enum OrcConf {
"If the number of distinct keys in a dictionary is greater than this\n" +
"fraction of the total number of non-null rows, turn off \n" +
"dictionary encoding. Use 1 to always use dictionary encoding."),
+ DICTIONARY_MAX_SIZE_IN_BYTES("orc.dictionary.maxSizeInBytes",
+ "hive.exec.orc.dictionary.maxSizeInBytes",
+ 16 * 1024 * 1024,
Review Comment:
Look at the default dictionary of Presto and Trino, which is also 16mb.
https://github.com/prestodb/presto/blob/master/presto-orc/src/main/java/com/facebook/presto/orc/OrcWriterOptions.java#L37
```java
public static final DataSize DEFAULT_DICTIONARY_MAX_MEMORY = new DataSize(16, MEGABYTE);
```
https://github.com/trinodb/trino/blob/master/lib/trino-orc/src/main/java/io/trino/orc/OrcWriterOptions.java#L52
```java
private static final DataSize DEFAULT_DICTIONARY_MAX_MEMORY = DataSize.of(16, MEGABYTE);
```
--
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.
To unsubscribe, e-mail: issues-unsubscribe@orc.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] ORC-1986: Trigger flush stripe for large input rows [orc]
Posted by "dongjoon-hyun (via GitHub)" <gi...@apache.org>.
dongjoon-hyun commented on code in PR #2371:
URL: https://github.com/apache/orc/pull/2371#discussion_r2323138542
##########
java/core/src/java/org/apache/orc/OrcConf.java:
##########
@@ -182,6 +187,9 @@ public enum OrcConf {
"added to all of the writers. Valid range is [1,10000] and is primarily meant for" +
"testing. Setting this too low may negatively affect performance."
+ " Use orc.stripe.row.count instead if the value larger than orc.stripe.row.count."),
+ STRIPE_SIZE_CHECK("orc.stripe.size.check", "hive.exec.orc.default.stripe.size.check",
+ 128L * 1024 * 1024,
Review Comment:
ditto.
--
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.
To unsubscribe, e-mail: issues-unsubscribe@orc.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] ORC-1986: Trigger flush stripe for large input rows [orc]
Posted by "wankunde (via GitHub)" <gi...@apache.org>.
wankunde commented on code in PR #2371:
URL: https://github.com/apache/orc/pull/2371#discussion_r2321162659
##########
java/core/src/java/org/apache/orc/impl/WriterImpl.java:
##########
@@ -325,9 +327,9 @@ public boolean checkMemory(double newScale) throws IOException {
}
private boolean checkMemory() throws IOException {
- if (rowsSinceCheck >= ROWS_PER_CHECK) {
+ long size = treeWriter.estimateMemory();
Review Comment:
* If rowsSinceCheck >= ROWS_PER_CHECK, the `estimateMemory` will be called the same with before
* If rowsSinceCheck < ROWS_PER_CHECK, the `estimateMemory` will be called `ROWS_PER_CHECK / VectorizedRowBatch.size` times, and `5000 / 1024 = 4` times by default, I think this is much smaller overhead in the overall 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.
To unsubscribe, e-mail: issues-unsubscribe@orc.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org