Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

feat(EntityService): batched transactions and ebean updates #8456

Merged
merged 43 commits into from
Sep 3, 2023
Merged

feat(EntityService): batched transactions and ebean updates #8456

merged 43 commits into from
Sep 3, 2023

Conversation

david-leifker
Copy link
Collaborator

@david-leifker david-leifker commented Jul 19, 2023 •
edited

  • Removes MAE producer code and logic around MAE in EntityService
  • Adds batching of database aspect operations in EntityService
  • Exposes the Async futures (and handles them) when producing to MCL topic
  • Update ebean library
  • Fix dropped writes with multi-threading when deadlocked
  • Multithreading test added, along with fake data generator
  • Only generate default aspects for UPSERT/CREATE change types

Checklist

  • The PR conforms to DataHub's Contributing Guideline (particularly Commit Message Format)
  • Links to related issues (if applicable)
  • Tests for the changes have been added/updated (if applicable)
  • Docs related to the changes have been added/updated (if applicable). If a new feature has been added a Usage Guide has been added for the same.
  • For any breaking change/potential downtime/deprecation/big changes an entry has been made in Updating DataHub

@github-actions github-actions bot added product PR or Issue related to the DataHub UI/UX devops PR or Issue related to DataHub backend & deployment labels Jul 19, 2023
@david-leifker
Merge remote-tracking branch 'origin/master' into ebean-batched-trans…
e795eca 
…actions

# Conflicts:
#	metadata-io/src/main/java/com/linkedin/metadata/entity/EntityServiceImpl.java
#	metadata-io/src/main/java/com/linkedin/metadata/entity/EntityUtils.java
#	metadata-io/src/test/java/com/linkedin/metadata/entity/EbeanEntityServiceTest.java
#	metadata-io/src/test/java/com/linkedin/metadata/entity/EntityServiceTest.java
#	metadata-service/openapi-servlet/src/main/java/io/datahubproject/openapi/util/MappingUtil.java
#	metadata-service/restli-servlet-impl/src/main/java/com/linkedin/metadata/resources/entity/AspectResource.java
#	metadata-service/restli-servlet-impl/src/test/java/com/linkedin/metadata/resources/entity/AspectResourceTest.java
#	metadata-service/services/src/main/java/com/linkedin/metadata/entity/DeleteEntityService.java
@vercel
Copy link

vercel bot commented Jul 23, 2023

Deployment failed with the following error:

The provided GitHub repository does not contain the requested branch or commit reference. Please ensure the repository is not empty.

@david-leifker david-leifker mentioned this pull request Aug 18, 2023
Closed
5 tasks
@david-leifker
Copy link
Collaborator Author

david-leifker commented Aug 28, 2023 •
edited

Create flag for PATCH - Only generate default aspects for UPSERT/CREATE change types
Consider key aspect + PATCH default enable

@david-leifker
Copy link
Collaborator Author

Create flag for PATCH - Only generate default aspects for UPSERT/CREATE change types Consider key aspect + PATCH default enable

Was going to implement a more nuanced solution, however for now simply adding back the legacy behavior for PATCH around generating default aspects.

@david-leifker david-leifker added merge-pending-ci A PR that has passed review and should be merged once CI is green. and removed merge-pending-ci A PR that has passed review and should be merged once CI is green. labels Aug 29, 2023
RyanHolstien
RyanHolstien approved these changes Aug 29, 2023
View reviewed changes
...aphql-core/src/main/java/com/linkedin/datahub/graphql/resolvers/mutate/util/DomainUtils.java Outdated
for (MetadataChangeProposal change : changes) {
entityService.ingestProposal(change, getAuditStamp(actor), false);
}
entityService.ingestProposal(AspectsBatchImpl.builder()
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks like all of these private static functions are the same, can we consolidate them?

metadata-io/src/test/java/com/linkedin/metadata/AspectUtilsTest.java Outdated Show resolved Hide resolved
@david-leifker david-leifker merged commit 1b79142 into datahub-project:master Sep 3, 2023
43 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@RyanHolstien RyanHolstien RyanHolstien approved these changes

@iprentic iprentic Awaiting requested review from iprentic

@jjoyce0510 jjoyce0510 Awaiting requested review from jjoyce0510

@shirshanka shirshanka Awaiting requested review from shirshanka

Assignees
No one assigned
Labels
devops PR or Issue related to DataHub backend & deployment product PR or Issue related to the DataHub UI/UX release-0.11.0
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
2 participants
@david-leifker @RyanHolstien