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

Ro replica object store integration #444

Merged
merged 1 commit into from Apr 11, 2020

Conversation

toly-kournik
Copy link
Contributor

@toly-kournik toly-kournik commented Mar 11, 2020

Introducing S3-compatible external object store support.
Co-authored-by: @salieri11
Co-authored-by: @toly-kournik
Key points:
S3:

  • concord::storage::s3::client with S3KeyGenerator
  • handing off state transfer messages in BCStateTransfer if used with remote object store

General:

  • kvbc::ReplicaImp implements IAppState
  • removed ReplicaImp::StorageWrapperForIdleMode

Copy link
Contributor

@salieri11 salieri11 left a comment

Choose a reason for hiding this comment

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

Looks good! Just need a clarification about wait()

kvbc/src/ReplicaImp.cpp Outdated Show resolved Hide resolved
Copy link
Contributor

@salieri11 salieri11 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!

@toly-kournik toly-kournik changed the title Ro replica WIP: Ro replica object store integration Mar 18, 2020
@toly-kournik toly-kournik force-pushed the ro-replica branch 5 times, most recently from 34c3159 to e49d754 Compare March 19, 2020 13:07
@toly-kournik toly-kournik force-pushed the ro-replica branch 2 times, most recently from bd83b5c to c6edf7c Compare March 26, 2020 16:43
@toly-kournik toly-kournik force-pushed the ro-replica branch 7 times, most recently from 8f93fbb to 3aee377 Compare April 1, 2020 15:00
@toly-kournik toly-kournik force-pushed the ro-replica branch 4 times, most recently from 9d36230 to 6095e0d Compare April 6, 2020 21:42
@toly-kournik toly-kournik changed the title WIP: Ro replica object store integration Ro replica object store integration Apr 6, 2020
Copy link
Contributor

@dartdart26 dartdart26 left a comment

Choose a reason for hiding this comment

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

Thanks for the offline discussion and the cleanup @toly-kournik ! I think it looks good and we can merge it as is. I have a few minor comments that I will leave and you can decide whether you want to address them in a separate PR.

@dartdart26
Copy link
Contributor

dartdart26 commented Apr 10, 2020

Thanks @toly-kournik and @salieri11 for the effort, good work! It is great that we can now save data to S3! And the code is clearer and simpler.

The commit includes new code that support S3 as possible implementation of IDBClient.
We plan to use it with ReadOnly replica for the replication of the data to external storage.

Introduce has(Key) method in IDBClient interface and implement it in all its derived classes

ROReplica: handoff state transfer messages to a dedicated thread to avoid process stuck in case of long blocking call to an external object store.
Re-factoring: replace various BCStateTran config params by their Config struct equivalent.

IDBclient:
- add has(Sliver key)

- ReplicaForStateTransfer owns the IStateTransfer* instance instead of kvbc::ReplicaImp
- remove RorAppState
- IAppState implemented by kvbc::ReplicaImp
- v1::DBAdapter
	- setLatestBlock()
	- setLastReachableBlock()
	- add an ability to add free keys
- ecs::S3Client -> s3::client

DBAdapter to use additional IDBClient for metadata
@toly-kournik toly-kournik reopened this Apr 11, 2020
@toly-kournik toly-kournik merged commit e23aec6 into vmware:master Apr 11, 2020
f-squirrel pushed a commit to f-squirrel/concord-bft that referenced this pull request Apr 17, 2020
Introducing S3-compatible external object store support.
Key points:
S3:
- concord::storage::s3::client with S3KeyGenerator
- handing off state transfer messages in BCStateTransfer if used with remote object store to avoid process stuck in case of long blocking call to an external object store.

General:
- kvbc::ReplicaImp implements IAppState
- removed ReplicaImp::StorageWrapperForIdleMode

Co-authored-by: Igor Golikov <salieri11@gmail.com>
Co-authored-by: @toly-kournik
@toly-kournik toly-kournik deleted the ro-replica branch June 12, 2020 11:30
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.

None yet

5 participants