Skip to content

Conversation

@YihuiGuo
Copy link
Contributor

@YihuiGuo YihuiGuo commented Jun 27, 2022

Based on #387
Add purview registry.

Keep sql registry code unchanged to avoid conflict.
Conform to Registy interfaces, provide same method and data format.

NOTICE that :
This PR does not totally eliminate the risk of concurrent conflict.
In old version, we update all entities like
purviewclient.upload_entities([entity_batch])
In this PR, several changes has been applied to shrink the time window for a concurrent race condition to happen:

  1. For entity uploading, only one entity was filled into the batch. (This means, register 1 entity for 1 upload)
  2. Check for entity existence before uploading by qualifeidName+typeName. If the matching entity exists, we will further check if these entities are identical, if not (an "update" operation), registry api will return HTTP409 Conflict.

Limitations:
By applying changes above to purview registry, the time window for a race condition to happen shrinks, but not eliminated.
A complete change needs discussion with Purview team about recommended way to secure MVCC.

@YihuiGuo YihuiGuo changed the title Purview Registry New Purview Registry adapt Sql data model Jul 4, 2022
Copy link
Collaborator

@Yuqing-cat Yuqing-cat left a comment

Choose a reason for hiding this comment

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

Please make more e2e test

@blrchen blrchen added the safe to test Tag to execute build pipeline for a PR from forked repo label Jul 4, 2022
@Yuqing-cat Yuqing-cat merged commit 7620fc7 into feathr-ai:main Jul 4, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

safe to test Tag to execute build pipeline for a PR from forked repo

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants