-
Notifications
You must be signed in to change notification settings - Fork 4.5k
Add DirtyBit to represent whether Counters have been committed. #219
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.
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
Conversation
4da51b9 to
99c76c1
Compare
|
R: @bjchambers |
| * committing() None COMMITTING None | ||
| * committed() None None COMMITTED | ||
| */ | ||
| @VisibleForTesting |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
DirtyBit suggest a bool, which this is explicitly not. CommitState?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
done
3e5d21c to
9b76b75
Compare
9b76b75 to
09f4097
Compare
|
LGTM |
604f5ab to
b7bd50c
Compare
| @Nullable | ||
| public abstract CounterMean<T> getMean(); | ||
|
|
||
| /** |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
/**
* Represents ...
(No need to say CommitState)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
done
dee5843 to
8973646
Compare
|
PTAL |
| /** | ||
| * Returns if the counter contains non-committed aggregate. | ||
| * | ||
| * <p>After all possible mutations have completed, the reader should check |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This looks great. Can you put the details of this counter lifecycle/contract in the Counter class javadoc?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
done
Hadoop tuning
Update `pbs_for_create`, `pbs_for_set_no_merge`, `pbs_for_set_with_merge`, and `pbs_for_update` to match semantics expected by current versions of [conformance tests](googleapis/conformance-tests@0bb8520): - Rather than create separate `Write.transform` messages to hold field transforms, inline them as `update_transforms` in the main `Write.update` message (which will always be created now). Copy in the current version of the conftest JSON files and verify. Closes apache#217
No description provided.