Skip to content

SAMZA-2709: Adding partial update api to Table API#1560

Merged
mynameborat merged 9 commits intoapache:masterfrom
ajothomas:PartialUpdatesUpstream
Jan 20, 2022
Merged

SAMZA-2709: Adding partial update api to Table API#1560
mynameborat merged 9 commits intoapache:masterfrom
ajothomas:PartialUpdatesUpstream

Conversation

@ajothomas
Copy link
Contributor

Feature:
Samza Table API currently supports PUT, GET and DELETE. Puts typically write/overwrite the data for a key. Users have frequently requested for the ability to perform partial updates i.e update select fields or a part of the record. This PR intends to add update to Table API.

Changes:

  • Added updateAsync and updateAllAsync methods to TableWriteFunction and to AsyncReadWriteTable
  • All implementations of AsyncReadWriteTable have been changed to accommodate the API change
  • Added sendUpdateTo method to MessageStream. Added corresponding operator spec and operator implementation as well

Tests:

  • Added tests for changes to all table implementations

API Changes

  • TableWriteFunction was earlier using K,V generic types to represent key and record type. With partial updates, U type has been added to represent partial updates which is a backward compatible change
  • Similarly, U update generic type was added to AsyncReadWriteTable class generic signature. It affects all implementing classes as well.

@ajothomas ajothomas force-pushed the PartialUpdatesUpstream branch 4 times, most recently from cd68e03 to 22c3c41 Compare November 17, 2021 18:19
Copy link
Contributor

@xinyuiscool xinyuiscool left a comment

Choose a reason for hiding this comment

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

Reviewed 1/3. Continue later.

@ajothomas ajothomas force-pushed the PartialUpdatesUpstream branch 2 times, most recently from c865bdc to e4ac3cd Compare November 29, 2021 22:23
@ajothomas ajothomas force-pushed the PartialUpdatesUpstream branch from e4ac3cd to 7e4ad6a Compare November 30, 2021 21:40
@ajothomas ajothomas force-pushed the PartialUpdatesUpstream branch 3 times, most recently from d14fe2e to 3d34c1f Compare December 2, 2021 01:20
@ajothomas ajothomas force-pushed the PartialUpdatesUpstream branch from 3d34c1f to abfa54f Compare December 2, 2021 19:33
Copy link
Contributor

@xinyuiscool xinyuiscool left a comment

Choose a reason for hiding this comment

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

The batching impl which is trying to cast the update to the value type is not correct. The fundamental problem is that Operation correctly only contains key and value. We need to add a Update there too to reflect the update operation. For put and get, the getUpdate() will return null. For UpdateOperation. we return null in getValue() but return the correct update in the getUpdate().

@ajothomas ajothomas force-pushed the PartialUpdatesUpstream branch from d409a09 to ebf1ec3 Compare December 3, 2021 00:36
@ajothomas ajothomas force-pushed the PartialUpdatesUpstream branch from ebf1ec3 to e259dfb Compare December 3, 2021 01:08
@ajothomas ajothomas force-pushed the PartialUpdatesUpstream branch from e259dfb to 50bca8f Compare December 6, 2021 18:51
@ajothomas ajothomas force-pushed the PartialUpdatesUpstream branch 2 times, most recently from 9e8c6be to fa906e5 Compare December 20, 2021 20:48
@ajothomas ajothomas force-pushed the PartialUpdatesUpstream branch from fa906e5 to b030b75 Compare January 13, 2022 23:09
Copy link
Contributor

@xinyuiscool xinyuiscool left a comment

Choose a reason for hiding this comment

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

Overall looks look. Some minor things in the comments.

@ajothomas ajothomas force-pushed the PartialUpdatesUpstream branch from aa44105 to 65ef2a2 Compare January 14, 2022 17:06
@ajothomas ajothomas force-pushed the PartialUpdatesUpstream branch from 65ef2a2 to fc8287d Compare January 14, 2022 18:19
Copy link
Contributor

@xinyuiscool xinyuiscool left a comment

Choose a reason for hiding this comment

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

LGTM! Thanks for all the improvements!

Copy link
Contributor

@mynameborat mynameborat left a comment

Choose a reason for hiding this comment

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

Minor follow up on the comments; Looks good otherwise.

@mynameborat mynameborat merged commit b56044a into apache:master Jan 20, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants