You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@hudi.apache.org by "parisni (via GitHub)" <gi...@apache.org> on 2024/03/08 22:22:05 UTC
[PR] [HUDI-7494]: multi writer sync partition to glue will missing some partitions [hudi]
parisni opened a new pull request, #10841:
URL: https://github.com/apache/hudi/pull/10841
Glue will miss some partitions when multiple writers are involved. This is related to #8745
and it fixes #8634
### Change Logs
_Describe context and summary for this change. Highlight if any code was copied._
### Impact
_Describe any public API or user-facing feature change or any performance impact._
### Risk level (write none, low medium or high below)
_If medium or high, explain what verification was done to mitigate the risks._
### Documentation Update
_Describe any necessary documentation update if there is any new feature, config, or user-facing change. If not, put "none"._
- _The config description must be updated if new configs are added or the default value of the configs are changed_
- _Any new feature or user-facing change requires updating the Hudi website. Please create a Jira ticket, attach the
ticket number here and follow the [instruction](https://hudi.apache.org/contribute/developer-setup#website) to make
changes to the website._
### Contributor's checklist
- [ ] Read through [contributor's guide](https://hudi.apache.org/contribute/how-to-contribute)
- [ ] Change Logs and Impact were stated clearly
- [ ] Adequate tests were added if applicable
- [ ] CI 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.
To unsubscribe, e-mail: commits-unsubscribe@hudi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [HUDI-7494]: multi writer sync partition to glue will missing some partitions [hudi]
Posted by "parisni (via GitHub)" <gi...@apache.org>.
parisni commented on code in PR #10841:
URL: https://github.com/apache/hudi/pull/10841#discussion_r1519527967
##########
hudi-aws/src/main/java/org/apache/hudi/aws/sync/AWSGlueCatalogSyncClient.java:
##########
@@ -689,28 +692,49 @@ public void createDatabase(String databaseName) {
public Option<String> getLastCommitTimeSynced(String tableName) {
try {
Table table = getTable(awsGlue, databaseName, tableName);
- return Option.ofNullable(table.parameters().get(HOODIE_LAST_COMMIT_TIME_SYNC));
+ return Option.ofNullable(table.parameters().getOrDefault(HOODIE_LAST_COMMIT_TIME_SYNC, null));
} catch (Exception e) {
throw new HoodieGlueSyncException("Fail to get last sync commit time for " + tableId(databaseName, tableName), e);
}
}
+ @Override
+ public Option<String> getLastCommitCompletionTimeSynced(String tableName) {
Review Comment:
hive sync tools uses it https://github.com/apache/hudi/blob/ff0e67f78dfde76cc7bc02ce9b9c11cba846197d/hudi-sync/hudi-hive-sync/src/main/java/org/apache/hudi/hive/HiveSyncTool.java#L263
--
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: commits-unsubscribe@hudi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [HUDI-7494]: multi writer sync partition to glue will missing some partitions [hudi]
Posted by "hudi-bot (via GitHub)" <gi...@apache.org>.
hudi-bot commented on PR #10841:
URL: https://github.com/apache/hudi/pull/10841#issuecomment-1988254138
<!--
Meta data
{
"version" : 1,
"metaDataEntries" : [ {
"hash" : "178a4ef27cbdcbc48fc4a0126b9f4ccc91dd2e3c",
"status" : "FAILURE",
"url" : "https://dev.azure.com/apache-hudi-ci-org/785b6ef4-2f42-4a89-8f0e-5f0d7039a0cc/_build/results?buildId=22853",
"triggerID" : "178a4ef27cbdcbc48fc4a0126b9f4ccc91dd2e3c",
"triggerType" : "PUSH"
}, {
"hash" : "66708318cb06e9711eaf5a682bff82c1838f2737",
"status" : "PENDING",
"url" : "https://dev.azure.com/apache-hudi-ci-org/785b6ef4-2f42-4a89-8f0e-5f0d7039a0cc/_build/results?buildId=22879",
"triggerID" : "66708318cb06e9711eaf5a682bff82c1838f2737",
"triggerType" : "PUSH"
} ]
}-->
## CI report:
* 178a4ef27cbdcbc48fc4a0126b9f4ccc91dd2e3c Azure: [FAILURE](https://dev.azure.com/apache-hudi-ci-org/785b6ef4-2f42-4a89-8f0e-5f0d7039a0cc/_build/results?buildId=22853)
* 66708318cb06e9711eaf5a682bff82c1838f2737 Azure: [PENDING](https://dev.azure.com/apache-hudi-ci-org/785b6ef4-2f42-4a89-8f0e-5f0d7039a0cc/_build/results?buildId=22879)
<details>
<summary>Bot commands</summary>
@hudi-bot supports the following commands:
- `@hudi-bot run azure` re-run the last Azure build
</details>
--
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: commits-unsubscribe@hudi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [HUDI-7494]: multi writer sync partition to glue will missing some partitions [hudi]
Posted by "parisni (via GitHub)" <gi...@apache.org>.
parisni commented on PR #10841:
URL: https://github.com/apache/hudi/pull/10841#issuecomment-2240106486
@codope @danny0405 May we consider merging this ? in our case it has fixed production problem.
--
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: commits-unsubscribe@hudi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [HUDI-7494]: multi writer sync partition to glue will missing some partitions [hudi]
Posted by "parisni (via GitHub)" <gi...@apache.org>.
parisni commented on code in PR #10841:
URL: https://github.com/apache/hudi/pull/10841#discussion_r1519531770
##########
hudi-aws/src/main/java/org/apache/hudi/aws/sync/AWSGlueCatalogSyncClient.java:
##########
@@ -689,28 +692,49 @@ public void createDatabase(String databaseName) {
public Option<String> getLastCommitTimeSynced(String tableName) {
try {
Table table = getTable(awsGlue, databaseName, tableName);
- return Option.ofNullable(table.parameters().get(HOODIE_LAST_COMMIT_TIME_SYNC));
+ return Option.ofNullable(table.parameters().getOrDefault(HOODIE_LAST_COMMIT_TIME_SYNC, null));
} catch (Exception e) {
throw new HoodieGlueSyncException("Fail to get last sync commit time for " + tableId(databaseName, tableName), e);
}
}
+ @Override
+ public Option<String> getLastCommitCompletionTimeSynced(String tableName) {
+ // Get the last commit completion time from the TBLproperties
+ try {
+ Table table = getTable(awsGlue, databaseName, tableName);
+ return Option.ofNullable(table.parameters().getOrDefault(HOODIE_LAST_COMMIT_COMPLETION_TIME_SYNC, null));
+ } catch (Exception e) {
+ throw new HoodieHiveSyncException("Failed to get the last commit completion time synced from the table " + tableName, e);
+ }
+ }
+
@Override
public void close() {
awsGlue.close();
}
@Override
public void updateLastCommitTimeSynced(String tableName) {
- if (!getActiveTimeline().lastInstant().isPresent()) {
+ HoodieTimeline activeTimeline = getActiveTimeline();
+ Option<String> lastCommitSynced = activeTimeline.lastInstant().map(HoodieInstant::getTimestamp);
+ Option<String> lastCommitCompletionSynced = activeTimeline
+ .getInstantsOrderedByCompletionTime()
+ .skip(activeTimeline.countInstants() - 1)
+ .findFirst()
+ .map(i -> Option.of(i.getCompletionTime()))
+ .orElse(Option.empty());
+ if (lastCommitSynced.isPresent()) {
+ try {
+ HashMap<String, String> propertyMap = new HashMap<>();
+ propertyMap.put(HOODIE_LAST_COMMIT_TIME_SYNC, lastCommitSynced.get());
+ propertyMap.put(HOODIE_LAST_COMMIT_COMPLETION_TIME_SYNC, lastCommitCompletionSynced.get());
Review Comment:
I just copied the hive sync code from https://github.com/apache/hudi/blob/ff0e67f78dfde76cc7bc02ce9b9c11cba846197d/hudi-sync/hudi-hive-sync/src/main/java/org/apache/hudi/hive/HoodieHiveSyncClient.java#L364-L363
BTW I agree an will propose a fix for both in later commit
--
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: commits-unsubscribe@hudi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [HUDI-7494]: multi writer sync partition to glue will missing some partitions [hudi]
Posted by "parisni (via GitHub)" <gi...@apache.org>.
parisni commented on PR #10841:
URL: https://github.com/apache/hudi/pull/10841#issuecomment-1988177685
> @parisni Thanks for the fix. Left a few comments. Also, consider adding a unit test for the scenario. You can mock Glue APIs.
Glue mock has been integrated recently via moto. However we don't have yet the tooling to mock efficiently the timeline and hudi ingestion in this module. I am working on this in https://github.com/apache/hudi/pull/10614
If you agree, let me test this patch in production and let you know
--
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: commits-unsubscribe@hudi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [HUDI-7494]: multi writer sync partition to glue will missing some partitions [hudi]
Posted by "hudi-bot (via GitHub)" <gi...@apache.org>.
hudi-bot commented on PR #10841:
URL: https://github.com/apache/hudi/pull/10841#issuecomment-1986524053
<!--
Meta data
{
"version" : 1,
"metaDataEntries" : [ {
"hash" : "178a4ef27cbdcbc48fc4a0126b9f4ccc91dd2e3c",
"status" : "PENDING",
"url" : "https://dev.azure.com/apache-hudi-ci-org/785b6ef4-2f42-4a89-8f0e-5f0d7039a0cc/_build/results?buildId=22853",
"triggerID" : "178a4ef27cbdcbc48fc4a0126b9f4ccc91dd2e3c",
"triggerType" : "PUSH"
} ]
}-->
## CI report:
* 178a4ef27cbdcbc48fc4a0126b9f4ccc91dd2e3c Azure: [PENDING](https://dev.azure.com/apache-hudi-ci-org/785b6ef4-2f42-4a89-8f0e-5f0d7039a0cc/_build/results?buildId=22853)
<details>
<summary>Bot commands</summary>
@hudi-bot supports the following commands:
- `@hudi-bot run azure` re-run the last Azure build
</details>
--
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: commits-unsubscribe@hudi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [HUDI-7494]: multi writer sync partition to glue will missing some partitions [hudi]
Posted by "parisni (via GitHub)" <gi...@apache.org>.
parisni commented on code in PR #10841:
URL: https://github.com/apache/hudi/pull/10841#discussion_r1524578375
##########
hudi-sync/hudi-hive-sync/src/main/java/org/apache/hudi/hive/HoodieHiveSyncClient.java:
##########
@@ -376,7 +376,9 @@ public void updateLastCommitTimeSynced(String tableName) {
SerDeInfo serdeInfo = sd.getSerdeInfo();
serdeInfo.putToParameters(ConfigUtils.TABLE_SERDE_PATH, basePath);
table.putToParameters(HOODIE_LAST_COMMIT_TIME_SYNC, lastCommitSynced.get());
- table.putToParameters(HOODIE_LAST_COMMIT_COMPLETION_TIME_SYNC, lastCommitCompletionSynced.get());
+ if (lastCommitCompletionSynced.isPresent()) {
+ table.putToParameters(HOODIE_LAST_COMMIT_COMPLETION_TIME_SYNC, lastCommitCompletionSynced.get());
+ }
Review Comment:
I'm afrait if not present, lastCommitCompletionSynced.get() would raise a NPE
--
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: commits-unsubscribe@hudi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [HUDI-7494]: multi writer sync partition to glue will missing some partitions [hudi]
Posted by "hudi-bot (via GitHub)" <gi...@apache.org>.
hudi-bot commented on PR #10841:
URL: https://github.com/apache/hudi/pull/10841#issuecomment-1988242129
<!--
Meta data
{
"version" : 1,
"metaDataEntries" : [ {
"hash" : "178a4ef27cbdcbc48fc4a0126b9f4ccc91dd2e3c",
"status" : "FAILURE",
"url" : "https://dev.azure.com/apache-hudi-ci-org/785b6ef4-2f42-4a89-8f0e-5f0d7039a0cc/_build/results?buildId=22853",
"triggerID" : "178a4ef27cbdcbc48fc4a0126b9f4ccc91dd2e3c",
"triggerType" : "PUSH"
}, {
"hash" : "66708318cb06e9711eaf5a682bff82c1838f2737",
"status" : "UNKNOWN",
"url" : "TBD",
"triggerID" : "66708318cb06e9711eaf5a682bff82c1838f2737",
"triggerType" : "PUSH"
} ]
}-->
## CI report:
* 178a4ef27cbdcbc48fc4a0126b9f4ccc91dd2e3c Azure: [FAILURE](https://dev.azure.com/apache-hudi-ci-org/785b6ef4-2f42-4a89-8f0e-5f0d7039a0cc/_build/results?buildId=22853)
* 66708318cb06e9711eaf5a682bff82c1838f2737 UNKNOWN
<details>
<summary>Bot commands</summary>
@hudi-bot supports the following commands:
- `@hudi-bot run azure` re-run the last Azure build
</details>
--
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: commits-unsubscribe@hudi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [HUDI-7494]: multi writer sync partition to glue will missing some partitions [hudi]
Posted by "codope (via GitHub)" <gi...@apache.org>.
codope merged PR #10841:
URL: https://github.com/apache/hudi/pull/10841
--
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: commits-unsubscribe@hudi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [HUDI-7494]: multi writer sync partition to glue will missing some partitions [hudi]
Posted by "hudi-bot (via GitHub)" <gi...@apache.org>.
hudi-bot commented on PR #10841:
URL: https://github.com/apache/hudi/pull/10841#issuecomment-1986564580
<!--
Meta data
{
"version" : 1,
"metaDataEntries" : [ {
"hash" : "178a4ef27cbdcbc48fc4a0126b9f4ccc91dd2e3c",
"status" : "FAILURE",
"url" : "https://dev.azure.com/apache-hudi-ci-org/785b6ef4-2f42-4a89-8f0e-5f0d7039a0cc/_build/results?buildId=22853",
"triggerID" : "178a4ef27cbdcbc48fc4a0126b9f4ccc91dd2e3c",
"triggerType" : "PUSH"
} ]
}-->
## CI report:
* 178a4ef27cbdcbc48fc4a0126b9f4ccc91dd2e3c Azure: [FAILURE](https://dev.azure.com/apache-hudi-ci-org/785b6ef4-2f42-4a89-8f0e-5f0d7039a0cc/_build/results?buildId=22853)
<details>
<summary>Bot commands</summary>
@hudi-bot supports the following commands:
- `@hudi-bot run azure` re-run the last Azure build
</details>
--
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: commits-unsubscribe@hudi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [HUDI-7494]: multi writer sync partition to glue will missing some partitions [hudi]
Posted by "parisni (via GitHub)" <gi...@apache.org>.
parisni commented on code in PR #10841:
URL: https://github.com/apache/hudi/pull/10841#discussion_r1524578824
##########
hudi-aws/src/main/java/org/apache/hudi/aws/sync/AWSGlueCatalogSyncClient.java:
##########
@@ -689,28 +692,51 @@ public void createDatabase(String databaseName) {
public Option<String> getLastCommitTimeSynced(String tableName) {
try {
Table table = getTable(awsGlue, databaseName, tableName);
- return Option.ofNullable(table.parameters().get(HOODIE_LAST_COMMIT_TIME_SYNC));
+ return Option.ofNullable(table.parameters().getOrDefault(HOODIE_LAST_COMMIT_TIME_SYNC, null));
} catch (Exception e) {
throw new HoodieGlueSyncException("Fail to get last sync commit time for " + tableId(databaseName, tableName), e);
}
}
+ @Override
+ public Option<String> getLastCommitCompletionTimeSynced(String tableName) {
+ // Get the last commit completion time from the TBLproperties
+ try {
+ Table table = getTable(awsGlue, databaseName, tableName);
+ return Option.ofNullable(table.parameters().getOrDefault(HOODIE_LAST_COMMIT_COMPLETION_TIME_SYNC, null));
+ } catch (Exception e) {
+ throw new HoodieHiveSyncException("Failed to get the last commit completion time synced from the table " + tableName, e);
Review Comment:
:+1:
--
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: commits-unsubscribe@hudi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [HUDI-7494]: multi writer sync partition to glue will missing some partitions [hudi]
Posted by "hudi-bot (via GitHub)" <gi...@apache.org>.
hudi-bot commented on PR #10841:
URL: https://github.com/apache/hudi/pull/10841#issuecomment-1997154770
<!--
Meta data
{
"version" : 1,
"metaDataEntries" : [ {
"hash" : "178a4ef27cbdcbc48fc4a0126b9f4ccc91dd2e3c",
"status" : "DELETED",
"url" : "https://dev.azure.com/apache-hudi-ci-org/785b6ef4-2f42-4a89-8f0e-5f0d7039a0cc/_build/results?buildId=22853",
"triggerID" : "178a4ef27cbdcbc48fc4a0126b9f4ccc91dd2e3c",
"triggerType" : "PUSH"
}, {
"hash" : "66708318cb06e9711eaf5a682bff82c1838f2737",
"status" : "SUCCESS",
"url" : "https://dev.azure.com/apache-hudi-ci-org/785b6ef4-2f42-4a89-8f0e-5f0d7039a0cc/_build/results?buildId=22879",
"triggerID" : "66708318cb06e9711eaf5a682bff82c1838f2737",
"triggerType" : "PUSH"
}, {
"hash" : "4fbd494da110add7e838496c996c7e1dd3d103c6",
"status" : "UNKNOWN",
"url" : "TBD",
"triggerID" : "4fbd494da110add7e838496c996c7e1dd3d103c6",
"triggerType" : "PUSH"
} ]
}-->
## CI report:
* 66708318cb06e9711eaf5a682bff82c1838f2737 Azure: [SUCCESS](https://dev.azure.com/apache-hudi-ci-org/785b6ef4-2f42-4a89-8f0e-5f0d7039a0cc/_build/results?buildId=22879)
* 4fbd494da110add7e838496c996c7e1dd3d103c6 UNKNOWN
<details>
<summary>Bot commands</summary>
@hudi-bot supports the following commands:
- `@hudi-bot run azure` re-run the last Azure build
</details>
--
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: commits-unsubscribe@hudi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [HUDI-7494]: multi writer sync partition to glue will missing some partitions [hudi]
Posted by "hudi-bot (via GitHub)" <gi...@apache.org>.
hudi-bot commented on PR #10841:
URL: https://github.com/apache/hudi/pull/10841#issuecomment-1997283261
<!--
Meta data
{
"version" : 1,
"metaDataEntries" : [ {
"hash" : "178a4ef27cbdcbc48fc4a0126b9f4ccc91dd2e3c",
"status" : "DELETED",
"url" : "https://dev.azure.com/apache-hudi-ci-org/785b6ef4-2f42-4a89-8f0e-5f0d7039a0cc/_build/results?buildId=22853",
"triggerID" : "178a4ef27cbdcbc48fc4a0126b9f4ccc91dd2e3c",
"triggerType" : "PUSH"
}, {
"hash" : "66708318cb06e9711eaf5a682bff82c1838f2737",
"status" : "DELETED",
"url" : "https://dev.azure.com/apache-hudi-ci-org/785b6ef4-2f42-4a89-8f0e-5f0d7039a0cc/_build/results?buildId=22879",
"triggerID" : "66708318cb06e9711eaf5a682bff82c1838f2737",
"triggerType" : "PUSH"
}, {
"hash" : "4fbd494da110add7e838496c996c7e1dd3d103c6",
"status" : "SUCCESS",
"url" : "https://dev.azure.com/apache-hudi-ci-org/785b6ef4-2f42-4a89-8f0e-5f0d7039a0cc/_build/results?buildId=22904",
"triggerID" : "4fbd494da110add7e838496c996c7e1dd3d103c6",
"triggerType" : "PUSH"
} ]
}-->
## CI report:
* 4fbd494da110add7e838496c996c7e1dd3d103c6 Azure: [SUCCESS](https://dev.azure.com/apache-hudi-ci-org/785b6ef4-2f42-4a89-8f0e-5f0d7039a0cc/_build/results?buildId=22904)
<details>
<summary>Bot commands</summary>
@hudi-bot supports the following commands:
- `@hudi-bot run azure` re-run the last Azure build
</details>
--
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: commits-unsubscribe@hudi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [HUDI-7494]: multi writer sync partition to glue will missing some partitions [hudi]
Posted by "hudi-bot (via GitHub)" <gi...@apache.org>.
hudi-bot commented on PR #10841:
URL: https://github.com/apache/hudi/pull/10841#issuecomment-1986517813
<!--
Meta data
{
"version" : 1,
"metaDataEntries" : [ {
"hash" : "178a4ef27cbdcbc48fc4a0126b9f4ccc91dd2e3c",
"status" : "UNKNOWN",
"url" : "TBD",
"triggerID" : "178a4ef27cbdcbc48fc4a0126b9f4ccc91dd2e3c",
"triggerType" : "PUSH"
} ]
}-->
## CI report:
* 178a4ef27cbdcbc48fc4a0126b9f4ccc91dd2e3c UNKNOWN
<details>
<summary>Bot commands</summary>
@hudi-bot supports the following commands:
- `@hudi-bot run azure` re-run the last Azure build
</details>
--
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: commits-unsubscribe@hudi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [HUDI-7494]: multi writer sync partition to glue will missing some partitions [hudi]
Posted by "hudi-bot (via GitHub)" <gi...@apache.org>.
hudi-bot commented on PR #10841:
URL: https://github.com/apache/hudi/pull/10841#issuecomment-1988444601
<!--
Meta data
{
"version" : 1,
"metaDataEntries" : [ {
"hash" : "178a4ef27cbdcbc48fc4a0126b9f4ccc91dd2e3c",
"status" : "DELETED",
"url" : "https://dev.azure.com/apache-hudi-ci-org/785b6ef4-2f42-4a89-8f0e-5f0d7039a0cc/_build/results?buildId=22853",
"triggerID" : "178a4ef27cbdcbc48fc4a0126b9f4ccc91dd2e3c",
"triggerType" : "PUSH"
}, {
"hash" : "66708318cb06e9711eaf5a682bff82c1838f2737",
"status" : "SUCCESS",
"url" : "https://dev.azure.com/apache-hudi-ci-org/785b6ef4-2f42-4a89-8f0e-5f0d7039a0cc/_build/results?buildId=22879",
"triggerID" : "66708318cb06e9711eaf5a682bff82c1838f2737",
"triggerType" : "PUSH"
} ]
}-->
## CI report:
* 66708318cb06e9711eaf5a682bff82c1838f2737 Azure: [SUCCESS](https://dev.azure.com/apache-hudi-ci-org/785b6ef4-2f42-4a89-8f0e-5f0d7039a0cc/_build/results?buildId=22879)
<details>
<summary>Bot commands</summary>
@hudi-bot supports the following commands:
- `@hudi-bot run azure` re-run the last Azure build
</details>
--
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: commits-unsubscribe@hudi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [HUDI-7494]: multi writer sync partition to glue will missing some partitions [hudi]
Posted by "codope (via GitHub)" <gi...@apache.org>.
codope commented on code in PR #10841:
URL: https://github.com/apache/hudi/pull/10841#discussion_r1518792330
##########
hudi-aws/src/main/java/org/apache/hudi/aws/sync/AWSGlueCatalogSyncClient.java:
##########
@@ -689,28 +692,49 @@ public void createDatabase(String databaseName) {
public Option<String> getLastCommitTimeSynced(String tableName) {
try {
Table table = getTable(awsGlue, databaseName, tableName);
- return Option.ofNullable(table.parameters().get(HOODIE_LAST_COMMIT_TIME_SYNC));
+ return Option.ofNullable(table.parameters().getOrDefault(HOODIE_LAST_COMMIT_TIME_SYNC, null));
} catch (Exception e) {
throw new HoodieGlueSyncException("Fail to get last sync commit time for " + tableId(databaseName, tableName), e);
}
}
+ @Override
+ public Option<String> getLastCommitCompletionTimeSynced(String tableName) {
+ // Get the last commit completion time from the TBLproperties
+ try {
+ Table table = getTable(awsGlue, databaseName, tableName);
+ return Option.ofNullable(table.parameters().getOrDefault(HOODIE_LAST_COMMIT_COMPLETION_TIME_SYNC, null));
+ } catch (Exception e) {
+ throw new HoodieHiveSyncException("Failed to get the last commit completion time synced from the table " + tableName, e);
+ }
+ }
+
@Override
public void close() {
awsGlue.close();
}
@Override
public void updateLastCommitTimeSynced(String tableName) {
- if (!getActiveTimeline().lastInstant().isPresent()) {
+ HoodieTimeline activeTimeline = getActiveTimeline();
+ Option<String> lastCommitSynced = activeTimeline.lastInstant().map(HoodieInstant::getTimestamp);
+ Option<String> lastCommitCompletionSynced = activeTimeline
+ .getInstantsOrderedByCompletionTime()
+ .skip(activeTimeline.countInstants() - 1)
+ .findFirst()
+ .map(i -> Option.of(i.getCompletionTime()))
+ .orElse(Option.empty());
+ if (lastCommitSynced.isPresent()) {
+ try {
+ HashMap<String, String> propertyMap = new HashMap<>();
+ propertyMap.put(HOODIE_LAST_COMMIT_TIME_SYNC, lastCommitSynced.get());
+ propertyMap.put(HOODIE_LAST_COMMIT_COMPLETION_TIME_SYNC, lastCommitCompletionSynced.get());
Review Comment:
Is there a case when `lastCommitCompletionSynced` can be empty? If so, it can throw `NoSuchElementException`. Should we also check `lastCommitCompletionSynced.isPresent()`?
##########
hudi-aws/src/main/java/org/apache/hudi/aws/sync/AWSGlueCatalogSyncClient.java:
##########
@@ -689,28 +692,49 @@ public void createDatabase(String databaseName) {
public Option<String> getLastCommitTimeSynced(String tableName) {
try {
Table table = getTable(awsGlue, databaseName, tableName);
- return Option.ofNullable(table.parameters().get(HOODIE_LAST_COMMIT_TIME_SYNC));
+ return Option.ofNullable(table.parameters().getOrDefault(HOODIE_LAST_COMMIT_TIME_SYNC, null));
} catch (Exception e) {
throw new HoodieGlueSyncException("Fail to get last sync commit time for " + tableId(databaseName, tableName), e);
}
}
+ @Override
+ public Option<String> getLastCommitCompletionTimeSynced(String tableName) {
Review Comment:
Where is this method called?
--
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: commits-unsubscribe@hudi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [HUDI-7494]: multi writer sync partition to glue will missing some partitions [hudi]
Posted by "CTTY (via GitHub)" <gi...@apache.org>.
CTTY commented on code in PR #10841:
URL: https://github.com/apache/hudi/pull/10841#discussion_r1524264366
##########
hudi-sync/hudi-hive-sync/src/main/java/org/apache/hudi/hive/HoodieHiveSyncClient.java:
##########
@@ -376,7 +376,9 @@ public void updateLastCommitTimeSynced(String tableName) {
SerDeInfo serdeInfo = sd.getSerdeInfo();
serdeInfo.putToParameters(ConfigUtils.TABLE_SERDE_PATH, basePath);
table.putToParameters(HOODIE_LAST_COMMIT_TIME_SYNC, lastCommitSynced.get());
- table.putToParameters(HOODIE_LAST_COMMIT_COMPLETION_TIME_SYNC, lastCommitCompletionSynced.get());
+ if (lastCommitCompletionSynced.isPresent()) {
+ table.putToParameters(HOODIE_LAST_COMMIT_COMPLETION_TIME_SYNC, lastCommitCompletionSynced.get());
+ }
Review Comment:
Checking if it's present shouldn't hurt but not sure if it's necessary. Setting it to null seems to have the same behavior as not setting it at all?
##########
hudi-aws/src/main/java/org/apache/hudi/aws/sync/AWSGlueCatalogSyncClient.java:
##########
@@ -689,28 +692,51 @@ public void createDatabase(String databaseName) {
public Option<String> getLastCommitTimeSynced(String tableName) {
try {
Table table = getTable(awsGlue, databaseName, tableName);
- return Option.ofNullable(table.parameters().get(HOODIE_LAST_COMMIT_TIME_SYNC));
+ return Option.ofNullable(table.parameters().getOrDefault(HOODIE_LAST_COMMIT_TIME_SYNC, null));
} catch (Exception e) {
throw new HoodieGlueSyncException("Fail to get last sync commit time for " + tableId(databaseName, tableName), e);
}
}
+ @Override
+ public Option<String> getLastCommitCompletionTimeSynced(String tableName) {
+ // Get the last commit completion time from the TBLproperties
+ try {
+ Table table = getTable(awsGlue, databaseName, tableName);
+ return Option.ofNullable(table.parameters().getOrDefault(HOODIE_LAST_COMMIT_COMPLETION_TIME_SYNC, null));
+ } catch (Exception e) {
+ throw new HoodieHiveSyncException("Failed to get the last commit completion time synced from the table " + tableName, e);
Review Comment:
We should use `HoodieGlueSyncException` here instead of `HoodieHiveSyncException`
--
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: commits-unsubscribe@hudi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [HUDI-7494]: multi writer sync partition to glue will missing some partitions [hudi]
Posted by "hudi-bot (via GitHub)" <gi...@apache.org>.
hudi-bot commented on PR #10841:
URL: https://github.com/apache/hudi/pull/10841#issuecomment-1997167511
<!--
Meta data
{
"version" : 1,
"metaDataEntries" : [ {
"hash" : "178a4ef27cbdcbc48fc4a0126b9f4ccc91dd2e3c",
"status" : "DELETED",
"url" : "https://dev.azure.com/apache-hudi-ci-org/785b6ef4-2f42-4a89-8f0e-5f0d7039a0cc/_build/results?buildId=22853",
"triggerID" : "178a4ef27cbdcbc48fc4a0126b9f4ccc91dd2e3c",
"triggerType" : "PUSH"
}, {
"hash" : "66708318cb06e9711eaf5a682bff82c1838f2737",
"status" : "SUCCESS",
"url" : "https://dev.azure.com/apache-hudi-ci-org/785b6ef4-2f42-4a89-8f0e-5f0d7039a0cc/_build/results?buildId=22879",
"triggerID" : "66708318cb06e9711eaf5a682bff82c1838f2737",
"triggerType" : "PUSH"
}, {
"hash" : "4fbd494da110add7e838496c996c7e1dd3d103c6",
"status" : "PENDING",
"url" : "https://dev.azure.com/apache-hudi-ci-org/785b6ef4-2f42-4a89-8f0e-5f0d7039a0cc/_build/results?buildId=22904",
"triggerID" : "4fbd494da110add7e838496c996c7e1dd3d103c6",
"triggerType" : "PUSH"
} ]
}-->
## CI report:
* 66708318cb06e9711eaf5a682bff82c1838f2737 Azure: [SUCCESS](https://dev.azure.com/apache-hudi-ci-org/785b6ef4-2f42-4a89-8f0e-5f0d7039a0cc/_build/results?buildId=22879)
* 4fbd494da110add7e838496c996c7e1dd3d103c6 Azure: [PENDING](https://dev.azure.com/apache-hudi-ci-org/785b6ef4-2f42-4a89-8f0e-5f0d7039a0cc/_build/results?buildId=22904)
<details>
<summary>Bot commands</summary>
@hudi-bot supports the following commands:
- `@hudi-bot run azure` re-run the last Azure build
</details>
--
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: commits-unsubscribe@hudi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [HUDI-7494]: multi writer sync partition to glue will missing some partitions [hudi]
Posted by "parisni (via GitHub)" <gi...@apache.org>.
parisni commented on PR #10841:
URL: https://github.com/apache/hudi/pull/10841#issuecomment-1998087773
I just tested in production and sadly this does not fix the issue. Not sure if it's specific to glue or if the general logic which fails to gather partitions of concurrent writes. Will have to dig
```
024-03-14 18:24:34 INFO org.apache.hudi.hive.HiveSyncTool: Last commit time synced was found to be 20240314182001597, last commit completion time is found to be 20240314182054000
2024-03-14 18:24:34 INFO org.apache.hudi.sync.common.HoodieSyncClient: Last commit time synced is 20240314182001597, Getting commits since then
2024-03-14 18:24:34 INFO org.apache.hudi.hive.HiveSyncTool: Storage partitions scan complete. Found 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: commits-unsubscribe@hudi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org