You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@hudi.apache.org by GitBox <gi...@apache.org> on 2021/03/10 05:37:32 UTC
[GitHub] [hudi] nsivabalan opened a new pull request #2653: [WIP] [Hudi 1615] Fixing null schema in bulk_insert row writer path
nsivabalan opened a new pull request #2653:
URL: https://github.com/apache/hudi/pull/2653
## *Tips*
- *Thank you very much for contributing to Apache Hudi.*
- *Please review https://hudi.apache.org/contributing.html before opening a pull request.*
## What is the purpose of the pull request
*(For example: This pull request adds quick-start document.)*
## Brief change log
*(for example:)*
- *Modify AnnotationLocation checkstyle rule in checkstyle.xml*
## Verify this pull request
*(Please pick either of the following options)*
This pull request is a trivial rework / code cleanup without any test coverage.
*(or)*
This pull request is already covered by existing tests, such as *(please describe tests)*.
(or)
This change added tests and can be verified as follows:
*(example:)*
- *Added integration tests for end-to-end.*
- *Added HoodieClientWriteTest to verify the change.*
- *Manually verified the change by running a job locally.*
## Committer checklist
- [ ] Has a corresponding JIRA in PR title & commit
- [ ] Commit message is descriptive of the change
- [ ] CI is green
- [ ] Necessary doc changes done or have another open PR
- [ ] For large changes, please consider breaking it into sub-tasks under an umbrella JIRA.
----------------------------------------------------------------
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
[GitHub] [hudi] nsivabalan commented on a change in pull request #2653: [WIP] [HUDI 1615] Fixing null schema in bulk_insert row writer path
Posted by GitBox <gi...@apache.org>.
nsivabalan commented on a change in pull request #2653:
URL: https://github.com/apache/hudi/pull/2653#discussion_r592751124
##########
File path: hudi-common/src/main/java/org/apache/hudi/common/util/CommitUtils.java
##########
@@ -66,7 +66,8 @@ public static HoodieCommitMetadata buildMetadata(List<HoodieWriteStat> writeStat
if (extraMetadata.isPresent()) {
extraMetadata.get().forEach(commitMetadata::addMetadata);
}
- commitMetadata.addMetadata(HoodieCommitMetadata.SCHEMA_KEY, schemaToStoreInCommit);
+ // do we need to set empty string for schema of schema is null ?
+ commitMetadata.addMetadata(HoodieCommitMetadata.SCHEMA_KEY, schemaToStoreInCommit == null ? "" : schemaToStoreInCommit);
Review comment:
do we need to set empty string for schema if schema is null ? Better to fix at the datasource layer to set the schema properly rather than silently setting empty string here.
----------------------------------------------------------------
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
[GitHub] [hudi] nsivabalan commented on a change in pull request #2653: [WIP] [HUDI 1615] Fixing null schema in bulk_insert row writer path
Posted by GitBox <gi...@apache.org>.
nsivabalan commented on a change in pull request #2653:
URL: https://github.com/apache/hudi/pull/2653#discussion_r592746307
##########
File path: hudi-spark-datasource/hudi-spark/src/test/scala/org/apache/hudi/functional/TestCOWDataSource.scala
##########
@@ -86,6 +85,43 @@ class TestCOWDataSource extends HoodieClientTestBase {
assertTrue(HoodieDataSourceHelpers.hasNewCommits(fs, basePath, "000"))
}
+ /**
+ * Test for https://issues.apache.org/jira/browse/HUDI-1615. Null Schema in BulkInsert row writer flow.
+ * This was reported by customer when archival kicks in as the schema in commit metadata is not set for bulk_insert
+ * row writer flow.
+ * In this test, we trigger a round of bulk_inserts and set archive related configs to be minimal. So, after 5 rounds,
+ * archival should kick in and 2 commits should be archived. If schema is valid, no exception will be thrown. If not,
+ * NPE will be thrown. Hence we don't have any explicit assertions.
+ */
+ @Test
+ def testArchivalIssue(): Unit = {
+ var structType : StructType = null
+ for (i <- 1 to 5) {
Review comment:
This test runs for ~ 2 mins in my mac. Not sure how else we can test this fix. any pointers are appreciated.
----------------------------------------------------------------
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
[GitHub] [hudi] vinothchandar merged pull request #2653: [HUDI 1615] Fixing null schema in bulk_insert row writer path
Posted by GitBox <gi...@apache.org>.
vinothchandar merged pull request #2653:
URL: https://github.com/apache/hudi/pull/2653
----------------------------------------------------------------
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
[GitHub] [hudi] codecov-io edited a comment on pull request #2653: [HUDI 1615] Fixing null schema in bulk_insert row writer path
Posted by GitBox <gi...@apache.org>.
codecov-io edited a comment on pull request #2653:
URL: https://github.com/apache/hudi/pull/2653#issuecomment-794938096
# [Codecov](https://codecov.io/gh/apache/hudi/pull/2653?src=pr&el=h1) Report
> Merging [#2653](https://codecov.io/gh/apache/hudi/pull/2653?src=pr&el=desc) (56759e8) into [master](https://codecov.io/gh/apache/hudi/commit/e8e6708aea1ace4aecaba594d7fd84a0b5cac635?el=desc) (e8e6708) will **decrease** coverage by `0.20%`.
> The diff coverage is `66.66%`.
[](https://codecov.io/gh/apache/hudi/pull/2653?src=pr&el=tree)
```diff
@@ Coverage Diff @@
## master #2653 +/- ##
============================================
- Coverage 51.99% 51.79% -0.21%
+ Complexity 3566 3503 -63
============================================
Files 465 459 -6
Lines 22187 21745 -442
Branches 2360 2338 -22
============================================
- Hits 11537 11263 -274
+ Misses 9649 9494 -155
+ Partials 1001 988 -13
```
| Flag | Coverage Δ | Complexity Δ | |
|---|---|---|---|
| hudicli | `37.01% <ø> (ø)` | `0.00 <ø> (ø)` | |
| hudiclient | `100.00% <ø> (ø)` | `0.00 <ø> (ø)` | |
| hudicommon | `51.48% <0.00%> (-0.06%)` | `0.00 <0.00> (ø)` | |
| hudiflink | `53.49% <ø> (ø)` | `0.00 <ø> (ø)` | |
| hudihadoopmr | `33.44% <ø> (ø)` | `0.00 <ø> (ø)` | |
| hudisparkdatasource | `70.87% <70.58%> (+1.02%)` | `0.00 <0.00> (ø)` | |
| hudisync | `49.62% <ø> (ø)` | `0.00 <ø> (ø)` | |
| huditimelineservice | `?` | `?` | |
| hudiutilities | `69.48% <ø> (ø)` | `0.00 <ø> (ø)` | |
Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags#carryforward-flags-in-the-pull-request-comment) to find out more.
| [Impacted Files](https://codecov.io/gh/apache/hudi/pull/2653?src=pr&el=tree) | Coverage Δ | Complexity Δ | |
|---|---|---|---|
| [.../java/org/apache/hudi/common/util/CommitUtils.java](https://codecov.io/gh/apache/hudi/pull/2653/diff?src=pr&el=tree#diff-aHVkaS1jb21tb24vc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2h1ZGkvY29tbW9uL3V0aWwvQ29tbWl0VXRpbHMuamF2YQ==) | `68.00% <0.00%> (-4.00%)` | `6.00 <0.00> (ø)` | |
| [...n/java/org/apache/hudi/internal/DefaultSource.java](https://codecov.io/gh/apache/hudi/pull/2653/diff?src=pr&el=tree#diff-aHVkaS1zcGFyay1kYXRhc291cmNlL2h1ZGktc3BhcmsyL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9odWRpL2ludGVybmFsL0RlZmF1bHRTb3VyY2UuamF2YQ==) | `0.00% <0.00%> (ø)` | `0.00 <0.00> (ø)` | |
| [...org/apache/hudi/spark3/internal/DefaultSource.java](https://codecov.io/gh/apache/hudi/pull/2653/diff?src=pr&el=tree#diff-aHVkaS1zcGFyay1kYXRhc291cmNlL2h1ZGktc3BhcmszL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9odWRpL3NwYXJrMy9pbnRlcm5hbC9EZWZhdWx0U291cmNlLmphdmE=) | `0.00% <0.00%> (ø)` | `0.00 <0.00> (ø)` | |
| [...n/scala/org/apache/hudi/HoodieSparkSqlWriter.scala](https://codecov.io/gh/apache/hudi/pull/2653/diff?src=pr&el=tree#diff-aHVkaS1zcGFyay1kYXRhc291cmNlL2h1ZGktc3Bhcmsvc3JjL21haW4vc2NhbGEvb3JnL2FwYWNoZS9odWRpL0hvb2RpZVNwYXJrU3FsV3JpdGVyLnNjYWxh) | `57.14% <80.00%> (+5.78%)` | `0.00 <0.00> (ø)` | |
| [...ache/hudi/common/fs/inline/InMemoryFileSystem.java](https://codecov.io/gh/apache/hudi/pull/2653/diff?src=pr&el=tree#diff-aHVkaS1jb21tb24vc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2h1ZGkvY29tbW9uL2ZzL2lubGluZS9Jbk1lbW9yeUZpbGVTeXN0ZW0uamF2YQ==) | `79.31% <0.00%> (-10.35%)` | `15.00% <0.00%> (-1.00%)` | |
| [...e/hudi/common/table/log/HoodieLogFormatWriter.java](https://codecov.io/gh/apache/hudi/pull/2653/diff?src=pr&el=tree#diff-aHVkaS1jb21tb24vc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2h1ZGkvY29tbW9uL3RhYmxlL2xvZy9Ib29kaWVMb2dGb3JtYXRXcml0ZXIuamF2YQ==) | `78.12% <0.00%> (-1.57%)` | `26.00% <0.00%> (ø%)` | |
| [...udi/timeline/service/handlers/BaseFileHandler.java](https://codecov.io/gh/apache/hudi/pull/2653/diff?src=pr&el=tree#diff-aHVkaS10aW1lbGluZS1zZXJ2aWNlL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9odWRpL3RpbWVsaW5lL3NlcnZpY2UvaGFuZGxlcnMvQmFzZUZpbGVIYW5kbGVyLmphdmE=) | | | |
| [...apache/hudi/timeline/service/handlers/Handler.java](https://codecov.io/gh/apache/hudi/pull/2653/diff?src=pr&el=tree#diff-aHVkaS10aW1lbGluZS1zZXJ2aWNlL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9odWRpL3RpbWVsaW5lL3NlcnZpY2UvaGFuZGxlcnMvSGFuZGxlci5qYXZh) | | | |
| [...di/timeline/service/handlers/FileSliceHandler.java](https://codecov.io/gh/apache/hudi/pull/2653/diff?src=pr&el=tree#diff-aHVkaS10aW1lbGluZS1zZXJ2aWNlL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9odWRpL3RpbWVsaW5lL3NlcnZpY2UvaGFuZGxlcnMvRmlsZVNsaWNlSGFuZGxlci5qYXZh) | | | |
| [...g/apache/hudi/timeline/service/RequestHandler.java](https://codecov.io/gh/apache/hudi/pull/2653/diff?src=pr&el=tree#diff-aHVkaS10aW1lbGluZS1zZXJ2aWNlL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9odWRpL3RpbWVsaW5lL3NlcnZpY2UvUmVxdWVzdEhhbmRsZXIuamF2YQ==) | | | |
| ... and [3 more](https://codecov.io/gh/apache/hudi/pull/2653/diff?src=pr&el=tree-more) | |
----------------------------------------------------------------
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
[GitHub] [hudi] nsivabalan commented on pull request #2653: [WIP] [HUDI 1615] Fixing null schema in bulk_insert row writer path
Posted by GitBox <gi...@apache.org>.
nsivabalan commented on pull request #2653:
URL: https://github.com/apache/hudi/pull/2653#issuecomment-797078556
@n3nash @vinothchandar : patch is ready to be reviewed.
----------------------------------------------------------------
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
[GitHub] [hudi] codecov-io edited a comment on pull request #2653: [HUDI 1615] Fixing null schema in bulk_insert row writer path
Posted by GitBox <gi...@apache.org>.
codecov-io edited a comment on pull request #2653:
URL: https://github.com/apache/hudi/pull/2653#issuecomment-794938096
# [Codecov](https://codecov.io/gh/apache/hudi/pull/2653?src=pr&el=h1) Report
> Merging [#2653](https://codecov.io/gh/apache/hudi/pull/2653?src=pr&el=desc) (56759e8) into [master](https://codecov.io/gh/apache/hudi/commit/e8e6708aea1ace4aecaba594d7fd84a0b5cac635?el=desc) (e8e6708) will **increase** coverage by `0.05%`.
> The diff coverage is `66.66%`.
[](https://codecov.io/gh/apache/hudi/pull/2653?src=pr&el=tree)
```diff
@@ Coverage Diff @@
## master #2653 +/- ##
============================================
+ Coverage 51.99% 52.05% +0.05%
+ Complexity 3566 3565 -1
============================================
Files 465 465
Lines 22187 22194 +7
Branches 2360 2362 +2
============================================
+ Hits 11537 11552 +15
+ Misses 9649 9635 -14
- Partials 1001 1007 +6
```
| Flag | Coverage Δ | Complexity Δ | |
|---|---|---|---|
| hudicli | `37.01% <ø> (ø)` | `0.00 <ø> (ø)` | |
| hudiclient | `100.00% <ø> (ø)` | `0.00 <ø> (ø)` | |
| hudicommon | `51.48% <0.00%> (-0.06%)` | `0.00 <0.00> (ø)` | |
| hudiflink | `53.49% <ø> (ø)` | `0.00 <ø> (ø)` | |
| hudihadoopmr | `33.44% <ø> (ø)` | `0.00 <ø> (ø)` | |
| hudisparkdatasource | `70.87% <70.58%> (+1.02%)` | `0.00 <0.00> (ø)` | |
| hudisync | `49.62% <ø> (ø)` | `0.00 <ø> (ø)` | |
| huditimelineservice | `64.36% <ø> (ø)` | `0.00 <ø> (ø)` | |
| hudiutilities | `69.48% <ø> (ø)` | `0.00 <ø> (ø)` | |
Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags#carryforward-flags-in-the-pull-request-comment) to find out more.
| [Impacted Files](https://codecov.io/gh/apache/hudi/pull/2653?src=pr&el=tree) | Coverage Δ | Complexity Δ | |
|---|---|---|---|
| [.../java/org/apache/hudi/common/util/CommitUtils.java](https://codecov.io/gh/apache/hudi/pull/2653/diff?src=pr&el=tree#diff-aHVkaS1jb21tb24vc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2h1ZGkvY29tbW9uL3V0aWwvQ29tbWl0VXRpbHMuamF2YQ==) | `68.00% <0.00%> (-4.00%)` | `6.00 <0.00> (ø)` | |
| [...n/java/org/apache/hudi/internal/DefaultSource.java](https://codecov.io/gh/apache/hudi/pull/2653/diff?src=pr&el=tree#diff-aHVkaS1zcGFyay1kYXRhc291cmNlL2h1ZGktc3BhcmsyL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9odWRpL2ludGVybmFsL0RlZmF1bHRTb3VyY2UuamF2YQ==) | `0.00% <0.00%> (ø)` | `0.00 <0.00> (ø)` | |
| [...org/apache/hudi/spark3/internal/DefaultSource.java](https://codecov.io/gh/apache/hudi/pull/2653/diff?src=pr&el=tree#diff-aHVkaS1zcGFyay1kYXRhc291cmNlL2h1ZGktc3BhcmszL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9odWRpL3NwYXJrMy9pbnRlcm5hbC9EZWZhdWx0U291cmNlLmphdmE=) | `0.00% <0.00%> (ø)` | `0.00 <0.00> (ø)` | |
| [...n/scala/org/apache/hudi/HoodieSparkSqlWriter.scala](https://codecov.io/gh/apache/hudi/pull/2653/diff?src=pr&el=tree#diff-aHVkaS1zcGFyay1kYXRhc291cmNlL2h1ZGktc3Bhcmsvc3JjL21haW4vc2NhbGEvb3JnL2FwYWNoZS9odWRpL0hvb2RpZVNwYXJrU3FsV3JpdGVyLnNjYWxh) | `57.14% <80.00%> (+5.78%)` | `0.00 <0.00> (ø)` | |
| [...ache/hudi/common/fs/inline/InMemoryFileSystem.java](https://codecov.io/gh/apache/hudi/pull/2653/diff?src=pr&el=tree#diff-aHVkaS1jb21tb24vc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2h1ZGkvY29tbW9uL2ZzL2lubGluZS9Jbk1lbW9yeUZpbGVTeXN0ZW0uamF2YQ==) | `79.31% <0.00%> (-10.35%)` | `15.00% <0.00%> (-1.00%)` | |
| [...e/hudi/common/table/log/HoodieLogFormatWriter.java](https://codecov.io/gh/apache/hudi/pull/2653/diff?src=pr&el=tree#diff-aHVkaS1jb21tb24vc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2h1ZGkvY29tbW9uL3RhYmxlL2xvZy9Ib29kaWVMb2dGb3JtYXRXcml0ZXIuamF2YQ==) | `78.12% <0.00%> (-1.57%)` | `26.00% <0.00%> (ø%)` | |
----------------------------------------------------------------
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
[GitHub] [hudi] codecov-io commented on pull request #2653: [WIP] [HUDI 1615] Fixing null schema in bulk_insert row writer path
Posted by GitBox <gi...@apache.org>.
codecov-io commented on pull request #2653:
URL: https://github.com/apache/hudi/pull/2653#issuecomment-794938096
# [Codecov](https://codecov.io/gh/apache/hudi/pull/2653?src=pr&el=h1) Report
> Merging [#2653](https://codecov.io/gh/apache/hudi/pull/2653?src=pr&el=desc) (86e7706) into [master](https://codecov.io/gh/apache/hudi/commit/26da4f546275e8ab6496537743efe73510cb723d?el=desc) (26da4f5) will **increase** coverage by `18.60%`.
> The diff coverage is `n/a`.
[](https://codecov.io/gh/apache/hudi/pull/2653?src=pr&el=tree)
```diff
@@ Coverage Diff @@
## master #2653 +/- ##
=============================================
+ Coverage 50.92% 69.53% +18.60%
+ Complexity 3168 364 -2804
=============================================
Files 433 53 -380
Lines 19812 1963 -17849
Branches 2033 235 -1798
=============================================
- Hits 10090 1365 -8725
+ Misses 8902 465 -8437
+ Partials 820 133 -687
```
| Flag | Coverage Δ | Complexity Δ | |
|---|---|---|---|
| hudicli | `?` | `?` | |
| hudiclient | `?` | `?` | |
| hudicommon | `?` | `?` | |
| hudiflink | `?` | `?` | |
| hudihadoopmr | `?` | `?` | |
| hudisparkdatasource | `?` | `?` | |
| hudisync | `?` | `?` | |
| huditimelineservice | `?` | `?` | |
| hudiutilities | `69.53% <ø> (+0.01%)` | `0.00 <ø> (ø)` | |
Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags#carryforward-flags-in-the-pull-request-comment) to find out more.
| [Impacted Files](https://codecov.io/gh/apache/hudi/pull/2653?src=pr&el=tree) | Coverage Δ | Complexity Δ | |
|---|---|---|---|
| [...hudi/utilities/sources/helpers/KafkaOffsetGen.java](https://codecov.io/gh/apache/hudi/pull/2653/diff?src=pr&el=tree#diff-aHVkaS11dGlsaXRpZXMvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2h1ZGkvdXRpbGl0aWVzL3NvdXJjZXMvaGVscGVycy9LYWZrYU9mZnNldEdlbi5qYXZh) | `85.84% <0.00%> (-2.94%)` | `20.00% <0.00%> (+4.00%)` | :arrow_down: |
| [...apache/hudi/utilities/deltastreamer/DeltaSync.java](https://codecov.io/gh/apache/hudi/pull/2653/diff?src=pr&el=tree#diff-aHVkaS11dGlsaXRpZXMvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2h1ZGkvdXRpbGl0aWVzL2RlbHRhc3RyZWFtZXIvRGVsdGFTeW5jLmphdmE=) | `70.34% <0.00%> (-0.52%)` | `53.00% <0.00%> (+2.00%)` | :arrow_down: |
| [...i/utilities/deltastreamer/HoodieDeltaStreamer.java](https://codecov.io/gh/apache/hudi/pull/2653/diff?src=pr&el=tree#diff-aHVkaS11dGlsaXRpZXMvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2h1ZGkvdXRpbGl0aWVzL2RlbHRhc3RyZWFtZXIvSG9vZGllRGVsdGFTdHJlYW1lci5qYXZh) | `68.72% <0.00%> (-0.26%)` | `18.00% <0.00%> (ø%)` | |
| [...s/deltastreamer/HoodieMultiTableDeltaStreamer.java](https://codecov.io/gh/apache/hudi/pull/2653/diff?src=pr&el=tree#diff-aHVkaS11dGlsaXRpZXMvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2h1ZGkvdXRpbGl0aWVzL2RlbHRhc3RyZWFtZXIvSG9vZGllTXVsdGlUYWJsZURlbHRhU3RyZWFtZXIuamF2YQ==) | `78.39% <0.00%> (ø)` | `18.00% <0.00%> (ø%)` | |
| [...rg/apache/hudi/cli/commands/SavepointsCommand.java](https://codecov.io/gh/apache/hudi/pull/2653/diff?src=pr&el=tree#diff-aHVkaS1jbGkvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2h1ZGkvY2xpL2NvbW1hbmRzL1NhdmVwb2ludHNDb21tYW5kLmphdmE=) | | | |
| [.../common/bloom/HoodieDynamicBoundedBloomFilter.java](https://codecov.io/gh/apache/hudi/pull/2653/diff?src=pr&el=tree#diff-aHVkaS1jb21tb24vc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2h1ZGkvY29tbW9uL2Jsb29tL0hvb2RpZUR5bmFtaWNCb3VuZGVkQmxvb21GaWx0ZXIuamF2YQ==) | | | |
| [...rg/apache/hudi/common/model/HoodieFileGroupId.java](https://codecov.io/gh/apache/hudi/pull/2653/diff?src=pr&el=tree#diff-aHVkaS1jb21tb24vc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2h1ZGkvY29tbW9uL21vZGVsL0hvb2RpZUZpbGVHcm91cElkLmphdmE=) | | | |
| [...apache/hudi/common/engine/HoodieEngineContext.java](https://codecov.io/gh/apache/hudi/pull/2653/diff?src=pr&el=tree#diff-aHVkaS1jb21tb24vc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2h1ZGkvY29tbW9uL2VuZ2luZS9Ib29kaWVFbmdpbmVDb250ZXh0LmphdmE=) | | | |
| [...a/org/apache/hudi/common/model/HoodieBaseFile.java](https://codecov.io/gh/apache/hudi/pull/2653/diff?src=pr&el=tree#diff-aHVkaS1jb21tb24vc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2h1ZGkvY29tbW9uL21vZGVsL0hvb2RpZUJhc2VGaWxlLmphdmE=) | | | |
| [...n/java/org/apache/hudi/common/HoodieCleanStat.java](https://codecov.io/gh/apache/hudi/pull/2653/diff?src=pr&el=tree#diff-aHVkaS1jb21tb24vc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2h1ZGkvY29tbW9uL0hvb2RpZUNsZWFuU3RhdC5qYXZh) | | | |
| ... and [372 more](https://codecov.io/gh/apache/hudi/pull/2653/diff?src=pr&el=tree-more) | |
----------------------------------------------------------------
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
[GitHub] [hudi] nsivabalan commented on a change in pull request #2653: [WIP] [HUDI 1615] Fixing null schema in bulk_insert row writer path
Posted by GitBox <gi...@apache.org>.
nsivabalan commented on a change in pull request #2653:
URL: https://github.com/apache/hudi/pull/2653#discussion_r592746307
##########
File path: hudi-spark-datasource/hudi-spark/src/test/scala/org/apache/hudi/functional/TestCOWDataSource.scala
##########
@@ -86,6 +85,43 @@ class TestCOWDataSource extends HoodieClientTestBase {
assertTrue(HoodieDataSourceHelpers.hasNewCommits(fs, basePath, "000"))
}
+ /**
+ * Test for https://issues.apache.org/jira/browse/HUDI-1615. Null Schema in BulkInsert row writer flow.
+ * This was reported by customer when archival kicks in as the schema in commit metadata is not set for bulk_insert
+ * row writer flow.
+ * In this test, we trigger a round of bulk_inserts and set archive related configs to be minimal. So, after 5 rounds,
+ * archival should kick in and 2 commits should be archived. If schema is valid, no exception will be thrown. If not,
+ * NPE will be thrown. Hence we don't have any explicit assertions.
+ */
+ @Test
+ def testArchivalIssue(): Unit = {
+ var structType : StructType = null
+ for (i <- 1 to 5) {
Review comment:
This test runs for ~ 45s in my mac. Not sure how else we can test this fix. any pointers are appreciated.
----------------------------------------------------------------
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
[GitHub] [hudi] nsivabalan commented on a change in pull request #2653: [WIP] [HUDI 1615] Fixing null schema in bulk_insert row writer path
Posted by GitBox <gi...@apache.org>.
nsivabalan commented on a change in pull request #2653:
URL: https://github.com/apache/hudi/pull/2653#discussion_r592745286
##########
File path: hudi-common/src/main/avro/HoodieCommitMetadata.avsc
##########
@@ -125,7 +125,8 @@
"name":"extraMetadata",
"type":["null", {
"type":"map",
- "values":"string"
+ "values":"string",
+ "default": null
Review comment:
Verified that this is backwards compatible.
----------------------------------------------------------------
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
[GitHub] [hudi] nsivabalan commented on a change in pull request #2653: [WIP] [HUDI 1615] Fixing null schema in bulk_insert row writer path
Posted by GitBox <gi...@apache.org>.
nsivabalan commented on a change in pull request #2653:
URL: https://github.com/apache/hudi/pull/2653#discussion_r591131225
##########
File path: hudi-spark-datasource/hudi-spark/src/main/scala/org/apache/hudi/HoodieSparkSqlWriter.scala
##########
@@ -286,30 +287,38 @@ private[hudi] object HoodieSparkSqlWriter {
basePath: Path,
path: Option[String],
instantTime: String): (Boolean, common.util.Option[String]) = {
- val structName = s"${tblName}_record"
- val nameSpace = s"hoodie.${tblName}"
- val writeConfig = DataSourceUtils.createHoodieConfig(null, path.get, tblName, mapAsJavaMap(parameters))
+ val sparkContext = sqlContext.sparkContext
+ // register classes & schemas
+ val (structName, nameSpace) = AvroConversionUtils.getAvroRecordNameAndNamespace(tblName)
+ sparkContext.getConf.registerKryoClasses(
Review comment:
Not sure if we really need to register the schema as we are not leveraging it for bulk insert w/ row writer.
----------------------------------------------------------------
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