Skip to content
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

YBSession API cleanup #8201

Closed
ttyusupov opened this issue Apr 27, 2021 · 0 comments
Closed

YBSession API cleanup #8201

ttyusupov opened this issue Apr 27, 2021 · 0 comments
Assignees
Labels
kind/enhancement This is an enhancement of an existing feature

Comments

@ttyusupov
Copy link
Contributor

The behavior of YBSession::GetAndClearPendingErrors is unpredictable when calling it before YBSession::Flush* and it is error-prone to use this function before session flush.

The goal is to update YBSession API to make it more clear and less error-prone.

@ttyusupov ttyusupov added the kind/enhancement This is an enhancement of an existing feature label Apr 27, 2021
@ttyusupov ttyusupov self-assigned this Apr 27, 2021
@ttyusupov ttyusupov changed the title Cleanup YBSession API YBSession API cleanup Apr 27, 2021
ttyusupov added a commit that referenced this issue Apr 28, 2021
Summary:
Behavior of `YBSession::GetAndClearPendingErrors` is unpredicable before calling `YBSession::Flush*` and it is error-prone to use this function before session flush.

Following changes have been made to clean up `YBSession` API and made it less error-prone:
- Added `FlushStatus` structure that contains the general status of session flush action and per-operation errors if occurred.
- Reworked `YBSession` to pass `FlushStatus` to flush callback instead of `Status`.
- Removed `YBSession::GetAndClearPendingErrors`
- Removed unused functions/parameters from `YBSession` interface.
- Functions that are only used from test code and shouldn't be used in production code have been renamed to `TEST_*`.
- Updated the code using `YBSession` according to API changes.

Proposed review order:
- session.h/session.cc
- yql/*
- table_handle.*
- tablet.*
- the rest (test classes)

Test Plan: - Jenkins

Reviewers: mbautin, sergei

Reviewed By: sergei

Subscribers: bogdan

Differential Revision: https://phabricator.dev.yugabyte.com/D11377
spolitov added a commit that referenced this issue Apr 29, 2021
Summary:
After landing the following non conflicting diffs master builds got broken:
7e12e38/D11377
300a1eb/D11366

This diff fixes the build.

Test Plan: Jenkins

Reviewers: dmitry, timur

Reviewed By: timur

Subscribers: ybase

Differential Revision: https://phabricator.dev.yugabyte.com/D11413
YintongMa pushed a commit to YintongMa/yugabyte-db that referenced this issue May 26, 2021
Summary:
Behavior of `YBSession::GetAndClearPendingErrors` is unpredicable before calling `YBSession::Flush*` and it is error-prone to use this function before session flush.

Following changes have been made to clean up `YBSession` API and made it less error-prone:
- Added `FlushStatus` structure that contains the general status of session flush action and per-operation errors if occurred.
- Reworked `YBSession` to pass `FlushStatus` to flush callback instead of `Status`.
- Removed `YBSession::GetAndClearPendingErrors`
- Removed unused functions/parameters from `YBSession` interface.
- Functions that are only used from test code and shouldn't be used in production code have been renamed to `TEST_*`.
- Updated the code using `YBSession` according to API changes.

Proposed review order:
- session.h/session.cc
- yql/*
- table_handle.*
- tablet.*
- the rest (test classes)

Test Plan: - Jenkins

Reviewers: mbautin, sergei

Reviewed By: sergei

Subscribers: bogdan

Differential Revision: https://phabricator.dev.yugabyte.com/D11377
YintongMa pushed a commit to YintongMa/yugabyte-db that referenced this issue May 26, 2021
Summary:
After landing the following non conflicting diffs master builds got broken:
7e12e38/D11377
300a1eb/D11366

This diff fixes the build.

Test Plan: Jenkins

Reviewers: dmitry, timur

Reviewed By: timur

Subscribers: ybase

Differential Revision: https://phabricator.dev.yugabyte.com/D11413
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind/enhancement This is an enhancement of an existing feature
Projects
None yet
Development

No branches or pull requests

1 participant