Skip to content

Conversation

@dmdashenkov
Copy link
Contributor

@dmdashenkov dmdashenkov commented Jan 27, 2020

This PR updates the API of the Cloud Datastore-based storage. In particular, we make it easier to work with transactions. The new methods of TransactionWrapper cover all the necessary database operations.

Also, in this PR, we update the format of InboxMessage storage. Previously, InboxMessages had a Timestamp property when_received. However, with the latest time-related changes in the framework, the precision of the Datastore Timestamp is now not enough for us. Thus, we introduce a new column received_at, which contains the number of nanoseconds, elapsed since the Unix epoch start. Though nanoseconds are, strictly speaking, a lie, this is good enough for our technical purposes.

As another side effect of the time-related changes, the standard idiom for querying entities by a time-based property has changed. Instead of passing an equals filter, it is recommended to

  1. pass an equals filter with a value rounded down to a whole microsecond;
  2. or pass a greater than - less than filter with a span of ± 1 second.

@dmdashenkov dmdashenkov self-assigned this Jan 27, 2020
@codecov
Copy link

codecov bot commented Jan 27, 2020

Codecov Report

Merging #143 into master will increase coverage by 0.14%.
The diff coverage is 96.84%.

@@             Coverage Diff              @@
##             master     #143      +/-   ##
============================================
+ Coverage     94.33%   94.48%   +0.14%     
+ Complexity      530      522       -8     
============================================
  Files            64       65       +1     
  Lines          1925     1904      -21     
  Branches        102       96       -6     
============================================
- Hits           1816     1799      -17     
  Misses           76       76              
+ Partials         33       29       -4

@dmdashenkov dmdashenkov marked this pull request as ready for review January 27, 2020 18:17
@dmdashenkov dmdashenkov requested a review from armiol January 27, 2020 18:18
@dmdashenkov
Copy link
Contributor Author

@armiol, PTAL.

@dmdashenkov
Copy link
Contributor Author

@armiol, PTAL.

Copy link
Contributor

@armiol armiol left a comment

Choose a reason for hiding this comment

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

@dmdashenkov LGTM with two minor comments to address.

return TimestampValue.of(fromProto(m.getWhenReceived()));
receivedAt("received_at", (m) -> {
Timestamp timestamp = m.getWhenReceived();
long epochNanos = timestamp.getSeconds() * NANOS_PER_SECOND + timestamp.getNanos();
Copy link
Contributor

Choose a reason for hiding this comment

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

Please use Timestamps.toNanos(..).

/**
* A Cloud Datastore transaction wrapper.
*
* @implNote The wrapper provides API for basic operations which can be done transactionally.
Copy link
Contributor

Choose a reason for hiding this comment

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

Please rephrase (as discussed) to emphasize on the Datastore limits.

@dmdashenkov dmdashenkov merged commit 47c9682 into master Jan 29, 2020
@dmdashenkov dmdashenkov deleted the expose-tx-api branch January 29, 2020 14:16
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