Skip to content

Support non-blocking, retryable record reading#4602

Closed
riversand963 wants to merge 1 commit intofacebook:masterfrom
riversand963:non_blocking_log_read
Closed

Support non-blocking, retryable record reading#4602
riversand963 wants to merge 1 commit intofacebook:masterfrom
riversand963:non_blocking_log_read

Conversation

@riversand963
Copy link
Copy Markdown
Contributor

@riversand963 riversand963 commented Oct 29, 2018

Add TryReadRecord method to log::Reader so that caller can call TryReadRecord
multiple times until a complete record is read. When a complete record is read,
TryReadRecord returns true; when a record has remaining part yet to be
read, or an error has occurred, TryReadRecord returns false. The caller can
implement different retry policies.

Also add unit test for non-blocking record read.

Example usage:

log::Reader reader(...);
Slice record;
std::string scratch;
while (reader.TryReadRecord(&record, &scratch)) {
  // process record
}
if (reader.reader_error_) {
 // handle error
} else {
 // consider retry
}

Note that reader_error_ is not exposed at the moment. Furthermore, we need
finer-grained error handling according to error type that is not exposed
either.

Test plan:

$make clean && make -j32 all check
$./log_test
$./log_test --gtest_filter=bool/RetriableLogTest.NonBlockingReadFullRecord/*

@riversand963 riversand963 added the WIP Work in progress label Oct 29, 2018
@riversand963 riversand963 self-assigned this Oct 29, 2018
@riversand963 riversand963 changed the title Support non-blocking, retryable record reading [WIP] Support non-blocking, retryable record reading Oct 29, 2018
@riversand963
Copy link
Copy Markdown
Contributor Author

riversand963 commented Oct 29, 2018

@siying and @miasantreble , could you take a look and see if this looks good? If so, I'll continue to add more tests.

p.s. I'm considering removing retry_after_eof because it sounds a little confusing combined with non-blocking.

Update:
I think retry_after_eof is still needed because we do not want to clear the buffer once we call ReadMore again after EOF.

Update on 10/31/2018:
I removed retry_after_eof from log::Reader because I think it is better to provide a non-blocking log read interface in log reader, and move anything related to retry to higher levels.

@riversand963 riversand963 force-pushed the non_blocking_log_read branch 2 times, most recently from bea41cf to 0005dc3 Compare October 31, 2018 00:36
@riversand963 riversand963 changed the title [WIP] Support non-blocking, retryable record reading Support non-blocking, retryable record reading Oct 31, 2018
@riversand963 riversand963 removed the WIP Work in progress label Oct 31, 2018
Copy link
Copy Markdown
Contributor

@facebook-github-bot facebook-github-bot left a comment

Choose a reason for hiding this comment

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

@riversand963 has imported this pull request. If you are a Facebook employee, you can view this diff on Phabricator.

@riversand963 riversand963 force-pushed the non_blocking_log_read branch from 348fa48 to a4eb6d6 Compare November 1, 2018 16:34
@facebook-github-bot
Copy link
Copy Markdown
Contributor

@riversand963 has updated the pull request. Re-import the pull request

@riversand963 riversand963 force-pushed the non_blocking_log_read branch from a4eb6d6 to 6c1df23 Compare November 1, 2018 22:46
@facebook-github-bot
Copy link
Copy Markdown
Contributor

@riversand963 has updated the pull request. Re-import the pull request

@riversand963 riversand963 force-pushed the non_blocking_log_read branch from 6c1df23 to 1da0c00 Compare November 1, 2018 22:51
@facebook-github-bot
Copy link
Copy Markdown
Contributor

@riversand963 has updated the pull request. Re-import the pull request

Copy link
Copy Markdown
Contributor

@facebook-github-bot facebook-github-bot left a comment

Choose a reason for hiding this comment

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

@riversand963 has imported this pull request. If you are a Facebook employee, you can view this diff on Phabricator.

@riversand963 riversand963 force-pushed the non_blocking_log_read branch from 1da0c00 to a7015ae Compare November 2, 2018 00:38
@facebook-github-bot
Copy link
Copy Markdown
Contributor

@riversand963 has updated the pull request. Re-import the pull request

Copy link
Copy Markdown
Contributor

@facebook-github-bot facebook-github-bot left a comment

Choose a reason for hiding this comment

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

@riversand963 has imported this pull request. If you are a Facebook employee, you can view this diff on Phabricator.

@riversand963 riversand963 force-pushed the non_blocking_log_read branch from a7015ae to b78e421 Compare November 3, 2018 02:36
@facebook-github-bot
Copy link
Copy Markdown
Contributor

@riversand963 has updated the pull request. Re-import the pull request

@facebook-github-bot
Copy link
Copy Markdown
Contributor

@riversand963 has updated the pull request. Re-import the pull request

Copy link
Copy Markdown
Contributor

@facebook-github-bot facebook-github-bot left a comment

Choose a reason for hiding this comment

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

@riversand963 has imported this pull request. If you are a Facebook employee, you can view this diff on Phabricator.

@facebook-github-bot
Copy link
Copy Markdown
Contributor

@riversand963 has updated the pull request. Re-import the pull request

Copy link
Copy Markdown
Contributor

@facebook-github-bot facebook-github-bot left a comment

Choose a reason for hiding this comment

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

@riversand963 has imported this pull request. If you are a Facebook employee, you can view this diff on Phabricator.

@facebook-github-bot
Copy link
Copy Markdown
Contributor

@riversand963 has updated the pull request. Re-import the pull request

Copy link
Copy Markdown
Contributor

@facebook-github-bot facebook-github-bot left a comment

Choose a reason for hiding this comment

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

@riversand963 has imported this pull request. If you are a Facebook employee, you can view this diff on Phabricator.

@facebook-github-bot
Copy link
Copy Markdown
Contributor

@riversand963 has updated the pull request. Re-import the pull request

Copy link
Copy Markdown
Contributor

@facebook-github-bot facebook-github-bot left a comment

Choose a reason for hiding this comment

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

@riversand963 has imported this pull request. If you are a Facebook employee, you can view this diff on Phabricator.

@riversand963 riversand963 force-pushed the non_blocking_log_read branch from a92d1ff to 6c445cd Compare December 6, 2018 23:59
@facebook-github-bot
Copy link
Copy Markdown
Contributor

@riversand963 has updated the pull request. Re-import the pull request

Copy link
Copy Markdown
Contributor

@facebook-github-bot facebook-github-bot left a comment

Choose a reason for hiding this comment

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

@riversand963 has imported this pull request. If you are a Facebook employee, you can view this diff on Phabricator.

@facebook-github-bot
Copy link
Copy Markdown
Contributor

@riversand963 has updated the pull request. Re-import the pull request

Add TryReadRecord method to log::Reader so that caller can call `TryReadRecord`
multiple times until a complete record is read. When a complete record is read,
`TryReadRecord` returns `true`; when a record has remaining part yet to be
read, or an error has occurred, `TryReadRecord` returns false. The caller can
implement different retry *policies*.

Also add unit test for non-blocking record read.

Example usage:
```
log::Reader reader(...);
Slice record;
std::string scratch;
while (reader.TryReadRecord(&record, &scratch)) {
  // process record
}
if (reader.reader_error_) {
 // handle error
} else {
 // consider retry
}
```
Note that `reader_error_` is not exposed at the moment. Furthermore, we need
finer-grained error handling according to error type that is not exposed
either.

Update log test and rebase.

Test plan:
```
$make clean && make -j32 all check
$./log_test
$./log_test --gtest_filter=bool/RetriableLogTest.NonBlockingReadFullRecord/*
```
@riversand963 riversand963 force-pushed the non_blocking_log_read branch from 7b43698 to 5b8452b Compare January 8, 2019 19:46
@facebook-github-bot
Copy link
Copy Markdown
Contributor

@riversand963 has updated the pull request. Re-import the pull request

Copy link
Copy Markdown
Contributor

@facebook-github-bot facebook-github-bot left a comment

Choose a reason for hiding this comment

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

@riversand963 has imported this pull request. If you are a Facebook employee, you can view this diff on Phabricator.

@riversand963
Copy link
Copy Markdown
Contributor Author

Moved to #4899

facebook-github-bot pushed a commit that referenced this pull request Mar 26, 2019
Summary:
This PR allows RocksDB to run in single-primary, multi-secondary process mode.
The writer is a regular RocksDB (e.g. an `DBImpl`) instance playing the role of a primary.
Multiple `DBImplSecondary` processes (secondaries) share the same set of SST files, MANIFEST, WAL files with the primary. Secondaries tail the MANIFEST of the primary and apply updates to their own in-memory state of the file system, e.g. `VersionStorageInfo`.

This PR has several components:
1. (Originally in #4745). Add a `PathNotFound` subcode to `IOError` to denote the failure when a secondary tries to open a file which has been deleted by the primary.

2. (Similar to #4602). Add `FragmentBufferedReader` to handle partially-read, trailing record at the end of a log from where future read can continue.

3. (Originally in #4710 and #4820). Add implementation of the secondary, i.e. `DBImplSecondary`.
3.1 Tail the primary's MANIFEST during recovery.
3.2 Tail the primary's MANIFEST during normal processing by calling `ReadAndApply`.
3.3 Tailing WAL will be in a future PR.

4. Add an example in 'examples/multi_processes_example.cc' to demonstrate the usage of secondary RocksDB instance in a multi-process setting. Instructions to run the example can be found at the beginning of the source code.
Pull Request resolved: #4899

Differential Revision: D14510945

Pulled By: riversand963

fbshipit-source-id: 4ac1c5693e6012ad23f7b4b42d3c374fecbe8886
jwasinger pushed a commit to jwasinger/rocksdb that referenced this pull request Apr 1, 2019
Summary:
This PR allows RocksDB to run in single-primary, multi-secondary process mode.
The writer is a regular RocksDB (e.g. an `DBImpl`) instance playing the role of a primary.
Multiple `DBImplSecondary` processes (secondaries) share the same set of SST files, MANIFEST, WAL files with the primary. Secondaries tail the MANIFEST of the primary and apply updates to their own in-memory state of the file system, e.g. `VersionStorageInfo`.

This PR has several components:
1. (Originally in facebook#4745). Add a `PathNotFound` subcode to `IOError` to denote the failure when a secondary tries to open a file which has been deleted by the primary.

2. (Similar to facebook#4602). Add `FragmentBufferedReader` to handle partially-read, trailing record at the end of a log from where future read can continue.

3. (Originally in facebook#4710 and facebook#4820). Add implementation of the secondary, i.e. `DBImplSecondary`.
3.1 Tail the primary's MANIFEST during recovery.
3.2 Tail the primary's MANIFEST during normal processing by calling `ReadAndApply`.
3.3 Tailing WAL will be in a future PR.

4. Add an example in 'examples/multi_processes_example.cc' to demonstrate the usage of secondary RocksDB instance in a multi-process setting. Instructions to run the example can be found at the beginning of the source code.
Pull Request resolved: facebook#4899

Differential Revision: D14510945

Pulled By: riversand963

fbshipit-source-id: 4ac1c5693e6012ad23f7b4b42d3c374fecbe8886
vagogte pushed a commit to vagogte/rocksdb that referenced this pull request Jun 18, 2019
Summary:
This PR allows RocksDB to run in single-primary, multi-secondary process mode.
The writer is a regular RocksDB (e.g. an `DBImpl`) instance playing the role of a primary.
Multiple `DBImplSecondary` processes (secondaries) share the same set of SST files, MANIFEST, WAL files with the primary. Secondaries tail the MANIFEST of the primary and apply updates to their own in-memory state of the file system, e.g. `VersionStorageInfo`.

This PR has several components:
1. (Originally in facebook#4745). Add a `PathNotFound` subcode to `IOError` to denote the failure when a secondary tries to open a file which has been deleted by the primary.

2. (Similar to facebook#4602). Add `FragmentBufferedReader` to handle partially-read, trailing record at the end of a log from where future read can continue.

3. (Originally in facebook#4710 and facebook#4820). Add implementation of the secondary, i.e. `DBImplSecondary`.
3.1 Tail the primary's MANIFEST during recovery.
3.2 Tail the primary's MANIFEST during normal processing by calling `ReadAndApply`.
3.3 Tailing WAL will be in a future PR.

4. Add an example in 'examples/multi_processes_example.cc' to demonstrate the usage of secondary RocksDB instance in a multi-process setting. Instructions to run the example can be found at the beginning of the source code.
Pull Request resolved: facebook#4899

Differential Revision: D14510945

Pulled By: riversand963

fbshipit-source-id: 4ac1c5693e6012ad23f7b4b42d3c374fecbe8886
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants