feat(Cassandra): add Cassandra implementation of EntityService #3286

xdl · 2021-09-22T15:35:38Z

This PR implements a new EntityService (DatastaxEntityService) and its associated DAO, aspect classes etc. in order to support Datastax's Cassandra for DataHub's document store component.

The implementation (Cassandra Datastax or Ebean SQL) can be set with the DAO_SERVICE_LAYER environment variable (e.g. in /docker/datahub-gms/env/docker.cassandra.env). The other corresponding DB configs that can be set are:

DATASTAX_DATASOURCE_USERNAME (e.g. cassandra)
DATASTAX_DATASOURCE_PASSWORD (e.g. cassandra)
DATASTAX_HOSTS (e.g. localhost)
DATASTAX_PORT (e.g. 9042)
DATASTAX_DATACENTER (e.g. datacenter1)
DATASTAX_KEYSPACE (e.g. datahubpoc)
DATASTAX_USE_SSL (e.g. false)

To test out locally, do a build and then run ./docker/dev-with-cassandra.sh.

Motivation

While evaluating DataHub for internal use, we identified that it was important for the query serving to be able to be horizontally scalable, as we've come up with scaling issues for one of our use cases relying on a traditional SQL store that DataHub would replace. Along with the requirement that the database should be highly available and with native support for multi DC replication, this led us to evaluate the use of Cassandra, a horizontally scalable database as part of our data catalog solution.

Technical Notes

Overview

We use the Cassandra library from testcontainers to provide an out-of-process Cassandra instance for integration tests (DatastaxEntityServiceTest.java)
The architecture (and tests) is inspired from the EbeanEntityService and the other Ebean implementations
Some nuances to queries, inserts and updates in a distributed database (e.g. see Paging and Sorting), are discussed below

Conditional Updates vs Transactions

Cassandra does not have ACID transactions; it does however have conditional updates which we make use of in DatastaxAspectDao.batchSaveLatestAspect and DatastaxAspectDao.condUpsertAspect to avoid a potential race condition when ingesting new entities (see #3231).

SCSI Support

Since Cassandra does not support JOINs, it would be difficult to implement SCSI without adding more columns into the metadata_aspect table, although it's our understanding that if this feature is about to be dropped, this wouldn't be a long-term issue.

Paging and Sorting

This mostly relates to the reimplementing of the listLatestAspects method. A new column in the DB has been added for the entity type (e.g. corpuser), populated upon insertion. The alternative to this would be to use a SASI, although there are some sources suggesting that it won't be any more performant (https://www.scnsoft.com/blog/cassandra-performance).

Furthermore, the sorting order (ascending by URN) feature has been dropped as it would have to be done in memory (since URN is used as the partition key), and this condition has been relaxed in testListLatestAspects.

Cassandra's paging has been implemented using the OffsetPager; there does not seem to be a performant way of getting the total number of matches back (e.g. https://www.datastax.com/blog/running-count-expensive-cassandra). Our use case isn't heavily focused on the frontend so there hasn't been much effort expended in optimising this.

Anticipated Further Work

Update to use DAO (https://docs.datastax.com/en/developer/java-driver/4.4/manual/mapper/daos/update/) class annotations which would be more idiomatic than the current implementation of the DatastaxAspect class
Implement and update to use EntityService.ingestProposal
Further investigation into rolling back (deleting) aspects
Optimisation around the batchGet method
Optimisation around frontend (listLatestAspects, listUrns) use cases

* Fix for datastax dao factory initialisation * Add readme for Cassandra quickstart

…eing

Cassandra more work

Move cassandra startup instructions to docker/cassandra/readme.md

Merge master into cassandra

EnricoMi · 2021-10-05T17:48:20Z

metadata-io/src/test/java/com/linkedin/metadata/entity/DatastaxEntityServiceTest.java

+import static org.testng.Assert.assertTrue;
+
+
+public class DatastaxEntityServiceTest {


This file is very similar to EbeanEntityServiceTest. The tests and common helper methods should be moved into a base test class and only implementation specific tests and code should be in these test classes that derive from the base.

Thanks Enrico and good spot, have moved those over to an BaseEntityServiceTest class and overrode the ones with different behaviour.

I named the base class for GraphServiceTests GraphServiceTestBase. So to be aligned with that, the class should be named EntitySearchTestBase.

https://github.com/linkedin/datahub/blob/master/metadata-io/src/test/java/com/linkedin/metadata/graph/GraphServiceTestBase.java.

merge upstream

…duce the number of contaners running at the same time

Change setup method lifecycles for test classes that use docker

merge upstream

restore gradle commands in build&test job

fix(ingestion): glue - delete CatalogId parameter from get_jobs api c…

[pull] master from G-Research:master

merge upstream

RyanHolstien

Changes since last review lgtm

merge upstream

metadata-io/src/test/java/com/linkedin/metadata/entity/EbeanEntityServiceTest.java

metadata-io/src/test/java/com/linkedin/metadata/graph/dgraph/DgraphGraphServiceTest.java

metadata-io/src/test/java/com/linkedin/metadata/search/LineageSearchServiceTest.java

jjoyce0510 · 2022-04-25T16:14:40Z

...a-service/factories/src/main/java/com/linkedin/gms/factory/entity/EbeanAspectDaoFactory.java


  @Bean(name = "ebeanAspectDao")
  @DependsOn({"gmsEbeanServiceConfig"})
+  @ConditionalOnProperty(name = "ENTITY_SERVICE_IMPL", havingValue = "ebean", matchIfMissing = true)


We've not used this annotation before -- where does this property come from? Env variables? Recently we've been trying to use application.yml config wherever possible

Yeah, it comes from the env. Now that you mention it, Cassandra config properties aren't in the application.yml at all.

Specifically it comes from the Spring env, which includes application.yml properties. If it's present in the application.yml it should still get picked up by ConditionalOnProperty

but in the application.yml we should be doing something like

entityService: impl: 'ebean' config ....

Added entityService.impl and cassandra config to the application.yml

jjoyce0510 · 2022-04-25T16:15:46Z

...ta-service/factories/src/main/java/com/linkedin/gms/factory/entity/EntityServiceFactory.java


  @Bean(name = "entityService")
-  @DependsOn({"ebeanAspectDao", "kafkaEventProducer", TopicConventionFactory.TOPIC_CONVENTION_BEAN, "entityRegistry"})
+  @DependsOn({"cassandraAspectDao", "kafkaEventProducer", TopicConventionFactory.TOPIC_CONVENTION_BEAN, "entityRegistry"})


Awesome. Same comment about the environment variable- I'd prefer if we could use the YAML

jjoyce0510 · 2022-04-25T16:16:32Z

...service/factories/src/main/java/com/linkedin/gms/factory/entity/RetentionServiceFactory.java

+  @ConditionalOnProperty(name = "ENTITY_SERVICE_IMPL", havingValue = "cassandra")
+  @Nonnull
+  protected RetentionService createCassandraInstance(CqlSession session) {
+    RetentionService retentionService = new CassandraRetentionService(_entityService, session, _batchSize);


Huge improvement to the retention service, to decouple from Ebean. Awesome work.

jjoyce0510 · 2022-04-25T16:17:31Z

...ce/factories/src/main/java/com/linkedin/metadata/boot/factories/BootstrapManagerFactory.java

@@ -27,7 +27,7 @@ public class BootstrapManagerFactory {
  @Qualifier("entityService")
  private EntityService _entityService;

-  @Autowired
+  @Autowired(required = false)


If this is not provided, will the IngestDataPlatformInstancesStep step break?

I don't think this even should be wired here. It doesn't get used directly in this class. If one of the beans used in this configuration class needs EbeanServer to be present, Spring should resolve it at that level through its dependency traversal (assuming it either wires it in or has some other form of DependsOn semantics)

Yeah it should. Each step should have its own factory but nonetheless the step shouldn't be depending on Ebean specifically. This seems dangerous

Missed it getting passed into the constructor here, yeah this will cause that step to fail if it's not present. We should pull this functionality out directly into the EntityService rather than have this step directly accessing the EbeanServer.

Yeah, it would be best if it just depended on the EntityService. Should it be sorted in this PR?

We have decided to address this on our side in a followup. No need to churn you kind folks on it.

jjoyce0510

Few final minor comments. Overall this looks ready to ship. Incredible work & greatly appreciate your patience with a fast-moving code base like ours

Cassandra datacenter naming consistency

add cassandra entity service configuration to application.yml

jjoyce0510

LGTM - Thank you guys for all the hard work.

…ub-project#3286)

Xiao Di Li and others added 20 commits September 15, 2021 09:22

Prelim cassandra push for code review

8422ac4

Removing white space

6b2f07e

Removing conditional updates for now

347cec7

Renaming table name, code cleanup

41c5e02

Remove sessionConfig map name

1c6d179

Add WIP PR writeup copy

4b30e0e

Removing some comments, fix indenting

ed1b75c

WIP tidyups, running on lab machine

b9f5335

Add listUrn implementation

f3d475a

dev-with-cassandra.sh and ingest example data example working. Also:

0da95c2

* Fix for datastax dao factory initialisation * Add readme for Cassandra quickstart

Move ebeanutils to entityutils

20f4f91

batching updates/inserts and tidying up

45605df

move strings from DAO into DatastaxAspect

fb85353

Tidying tests; minor DAO refactor

94b6d7c

Merge remote-tracking branch 'upstream/master' into cassandra-more-work

eea5725

simplify batchGet method and remove pagination support for the time b…

fa585f8

…eing

Using ConditionalOnProperty instead of instantiating null

83e9d7d

Merge pull request #1 from xdl/cassandra-more-work

a3d31fe

Cassandra more work

Move cassandra startup instructions to docker/cassandra/readme.md

5afb079

Merge pull request #2 from xdl/cassandra-more-work

a746866

Move cassandra startup instructions to docker/cassandra/readme.md

jgiannuzzi force-pushed the cassandra branch from d94f987 to a746866 Compare October 5, 2021 12:03

jgiannuzzi and others added 3 commits October 5, 2021 13:09

Merge pull request #6 from linkedin/master

a21bc99

Merge master into cassandra

Remove prints from EntityServiceFactory

2d8664a

Fix checkstyle warnings

2f18081

EnricoMi reviewed Oct 5, 2021

View reviewed changes

Eddie Li added 3 commits October 6, 2021 11:26

fix metdata-io:checkstyleMain violations

12574c4

Refactor common entityservice tests into a base class

933d8cd

Rename bean to entityAspect

c71c9d7

xdl mentioned this pull request Oct 7, 2021

Cassandra refactor G-Research/datahub#13

Merged

Merge remote-tracking branch 'gresearch/master' into cassandra-refactor

7f36f01

mattmatravers and others added 12 commits April 19, 2022 13:51

Merge pull request #109 from justinas-marozas/cassandra

947afff

merge upstream

change setup method lifecycles for test classes that use docker to re…

dfdf592

…duce the number of contaners running at the same time

Merge pull request #111 from justinas-marozas/cassandra

c24edf2

Change setup method lifecycles for test classes that use docker

Merge branch 'master' into cassandra

2051795

Merge pull request #113 from justinas-marozas/cassandra

da866ac

merge upstream

restore the gradle commands in build&test job

c0064f3

Merge branch 'G-Research:cassandra' into cassandra

11fc03c

Merge pull request #114 from justinas-marozas/cassandra

da5a4fb

restore gradle commands in build&test job

Merge pull request #115 from datahub-project/master

3fbced7

fix(ingestion): glue - delete CatalogId parameter from get_jobs api c…

Merge pull request #15 from G-Research/master

01d29ee

[pull] master from G-Research:master

Merge branch 'master' into cassandra

cf5d9dc

Merge pull request #118 from justinas-marozas/cassandra

738f499

merge upstream

RyanHolstien approved these changes Apr 22, 2022

View reviewed changes

justinas-marozas and others added 3 commits April 25, 2022 09:56

Merge branch 'G-Research:master' into master

6b78ff8

Merge branch 'master' into cassandra

639465f

Merge pull request #122 from justinas-marozas/cassandra

49e2ac2

merge upstream

jjoyce0510 reviewed Apr 25, 2022

View reviewed changes

metadata-io/src/test/java/com/linkedin/metadata/entity/EbeanEntityServiceTest.java Show resolved Hide resolved

jjoyce0510 reviewed Apr 25, 2022

View reviewed changes

metadata-io/src/test/java/com/linkedin/metadata/graph/dgraph/DgraphGraphServiceTest.java Show resolved Hide resolved

jjoyce0510 reviewed Apr 25, 2022

View reviewed changes

metadata-io/src/test/java/com/linkedin/metadata/search/LineageSearchServiceTest.java Show resolved Hide resolved

jjoyce0510 reviewed Apr 25, 2022

View reviewed changes

Justin Marozas and others added 3 commits April 25, 2022 19:01

add cassandra entity service configuration to application.yml

aeecc45

Apply suggestions from code review

3000b0f

Cassandra datacenter naming consistency

Merge pull request #123 from justinas-marozas/cassandra

778fc94

add cassandra entity service configuration to application.yml

jjoyce0510 approved these changes Apr 25, 2022

View reviewed changes

jjoyce0510 merged commit 01a5b13 into datahub-project:master Apr 26, 2022

maggiehays pushed a commit to maggiehays/datahub that referenced this pull request Aug 1, 2022

feat(Cassandra): Add Cassandra implementation of EntityService (datah…

b187eef

…ub-project#3286)

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat(Cassandra): add Cassandra implementation of EntityService #3286

feat(Cassandra): add Cassandra implementation of EntityService #3286

xdl commented Sep 22, 2021

EnricoMi Oct 5, 2021 •

edited

xdl Oct 7, 2021

EnricoMi Oct 7, 2021

RyanHolstien left a comment

jjoyce0510 Apr 25, 2022

justinas-marozas Apr 25, 2022

RyanHolstien Apr 25, 2022

jjoyce0510 Apr 25, 2022

justinas-marozas Apr 25, 2022

jjoyce0510 Apr 25, 2022

jjoyce0510 Apr 25, 2022

jjoyce0510 Apr 25, 2022

RyanHolstien Apr 25, 2022 •

edited

jjoyce0510 Apr 25, 2022

RyanHolstien Apr 25, 2022

justinas-marozas Apr 25, 2022

jjoyce0510 Apr 26, 2022

jjoyce0510 left a comment

jjoyce0510 left a comment

		import static org.testng.Assert.assertTrue;


		public class DatastaxEntityServiceTest {

feat(Cassandra): add Cassandra implementation of EntityService #3286

feat(Cassandra): add Cassandra implementation of EntityService #3286

Conversation

xdl commented Sep 22, 2021

Motivation

Technical Notes

Overview

Conditional Updates vs Transactions

SCSI Support

Paging and Sorting

Anticipated Further Work

EnricoMi Oct 5, 2021 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

RyanHolstien left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

RyanHolstien Apr 25, 2022 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jjoyce0510 left a comment

Choose a reason for hiding this comment

jjoyce0510 left a comment

Choose a reason for hiding this comment

EnricoMi Oct 5, 2021 •

edited

RyanHolstien Apr 25, 2022 •

edited