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

BP-30: BookKeeper Table Service #1205

Closed
4 of 5 tasks
sijie opened this issue Feb 26, 2018 · 10 comments
Closed
4 of 5 tasks

BP-30: BookKeeper Table Service #1205

sijie opened this issue Feb 26, 2018 · 10 comments
Labels
area/tableservice changes related to table service BP type/proposal
Milestone

Comments

@sijie
Copy link
Member

sijie commented Feb 26, 2018

BP

This is the master ticket for tracking BP-30 :

This BP is proposing adding a table (aka key/value) service component as a contrib module to the bookkeeper, which can later be used for metadata storage. This BP together with other BPs forms the ideas of improving metadata management in bookkeeper.

BP Details: https://docs.google.com/document/d/155xAwWv5IdOitHh1NVMEwCMGgB28M3FyMiQSxEpjE-Y/edit#heading=h.56rbh52koe3f

Proposal PR - #1185

Tasks:

@sijie sijie added the area/tableservice changes related to table service label Feb 27, 2018
sijie added a commit that referenced this issue Mar 4, 2018
Descriptions of the changes in this PR:

- update the pom files to integrate table service with bookkeeper build
- add 3 flags to control building/testing table service. by default the table service will be built and tested with existing maven commands.
  It only builds and runs the tests on following conditions:
    * `-Dstream`: enable building the table service
    * `-DstreamTests`: run the unit tests of table service
    * `-DstreamIntegrationTests`: run the integration tests of table service
- update CI jobs
    * Travis: compile table service for all builds, run unit tests if code modifications happen under `stream` directory
    * Jenkins: compile table service on precommit jobs, compile and run unit tests for postcommit jobs

Master Issue: #1205

Author: Sijie Guo <sijie@apache.org>

Reviewers: Jia Zhai <zhaijia@apache.org>

This closes #1216 from sijie/built_table_service
sijie added a commit that referenced this issue Mar 4, 2018
Descriptions of the changes in this PR:

This is a change for #1205

*Motivation*

in order to implify the other project to use table service, it would be good to make sure the client include table service related jars.

*Solution*

Include table service related jars in the client-all jar.

Master Issue: #1205

Author: Sijie Guo <sijie@apache.org>

Reviewers: Jia Zhai <None>

This closes #1226 from sijie/publish_table_service_jars
@sijie sijie added this to the 4.7.0 milestone Mar 12, 2018
@sijie sijie modified the milestones: 4.7.0, 4.8.0 Apr 10, 2018
sijie added a commit that referenced this issue May 22, 2018
Descriptions of the changes in this PR:

*Motivation*

table service was built over bookkeeper ledgers. it is an extension to bookies' functionalities, so it is much convenient to start the service as one additional component with bookie, just like how we start `AutoRecovery` along with bookie.

*Solution*

- include `stream/server` module as part of bookkeeper server/all binary distributions
- abstract `StorageServer` to allow controlling whether to start bookie or not
- provide a ServerLifecycleComponent of `StorageServer`, so it can be used as an extra component of bookie

*Tests*

- improve the integration tests to start table service as part of bookie containers
- move `LocationClientTest` to container based integration tests

Master Issue: #1205

Author: Sijie Guo <sijie@apache.org>

Reviewers: Enrico Olivelli <eolivelli@gmail.com>, Jia Zhai <None>

This closes #1422 from sijie/start_table_service
sijie added a commit that referenced this issue May 23, 2018
…n` to `tests/integration/cluster`

Descriptions of the changes in this PR:

The original integration tests were written based a non-dockerized standalone stream cluster. Moved them to use
the dockerized integration test framework. So all the integration tests are actually testing the table service run as part of bookies.

This change is based on #1422 .
a371ff2 is the change in this PR to be reviewed.

Master Issue: #1205

Author: Sijie Guo <sijie@apache.org>

Reviewers: Jia Zhai <None>

This closes #1423 from sijie/move_more_stream_it_tests
sijie added a commit that referenced this issue May 23, 2018
Descriptions of the changes in this PR:

*Motivation*

Current almost every grpc requests are wrapped into `StorageContainerRequest` and their responses
are wrapped into `StorageContainerResponse`. It makes things a bit complicated on adding new grpc
services.

To simplify things, we can use grpc ClientInterceptor to stamp container information (e.g. scId)
into the request metadata and write a grpc service registry to take the container information from
request metadata and dispatch requests to containers.

In order to achieve it, we need to move the grpc services to storage container.

*Solution*

This PR moves grpc services from server module to storage module, so we can simplify the wire protocols.

Master Issue: #1205

Author: Sijie Guo <sijie@apache.org>

Reviewers: Enrico Olivelli <eolivelli@gmail.com>, Jia Zhai <None>

This closes #1428 from sijie/move_grpc_service_to_storage
sijie added a commit that referenced this issue May 25, 2018
Descriptions of the changes in this PR:

This is a subsequent change after #1428.

*Motivation*

Current almost every grpc requests are wrapped into `StorageContainerRequest` and their responses
are wrapped into `StorageContainerResponse`. It makes things a bit complicated on adding new grpc
services.

*Changes*

To simplify things, this PR introduces two functionalities for simplifying dispatching container requests/responses.

1) *StorageContainerClientInterceptor*: A grpc `ClientInterceptor` that stamps container information (currently is `scId`) into the requests' metadata before sending the requests to the wire.

2) A simple grpc reverse proxy to dispatch grpc requests to the channels provided by a `ChannelFinder`.

*Tests*

1. Existing unit tests covered client interceptor changes.
2. Introduced a `stream-storage-tests-common` module to include common classes that would be used for testing.
3. Introduced a `PingPongService` for testing reverse proxy : unary/client-streaming/server-streaming/bidi-streaming.

Master Issue: #1205

Author: Sijie Guo <sijie@apache.org>

Reviewers: Jia Zhai <None>

This closes #1430 from sijie/interceptor_container_requests
sijie added a commit that referenced this issue May 29, 2018
…uests

Descriptions of the changes in this PR:

*Motivation*

#1430 introduced client interceptor and grpc reverse proxy for storage container requests. This PR is the subsequent change to leverage client interceptor and reverse proxy for storage container requests.

*Changes*

**Client Changes**

Changed `StorageContainerChannel` to add client interceptor to stamp storage container id into the the requests' metadata.

**Server Changes**

- moved the stream storage logic out of storage containers to a `service` package. the main logic will be kept in `RangeStoreService` and `RangeStoreServiceFactory`.
- make the storage container logic for hosting `StorageContainerService`.
- Each storage container will run an inprogress grpc server for serving the grpc services registered to the container and provide an `channel` for accessing those grpc service.
- Changed the server to use reverse proxy for serving/proxying storage container requests.

*NOTE*

This change doesn't directly remove `StorageContainerRequest` and `StorageContainerResponse`.  It would be done in a subsequent change.

Master Issue: #1205

Author: Sijie Guo <sijie@apache.org>

Reviewers: Jia Zhai <None>

This closes #1448 from sijie/storage_container_channel
sijie added a commit to sijie/bookkeeper that referenced this issue May 29, 2018
…sponse

*Motivation*

 apache#1448 uses reverse proxy for serving storage container requests and responses. so the `StorageContainerRequest` and `StorageContainerResponse`
 become redundant.

*Solution*

removes `StorageContainerRequest` and `StorageContainerResponse`

Master Issue: apache#1205
sijie added a commit that referenced this issue May 30, 2018
…sponse

Descriptions of the changes in this PR:

*Motivation*

 #1448 uses reverse proxy for serving storage container requests and responses. so the `StorageContainerRequest` and `StorageContainerResponse`
 become redundant.

*Solution*

removes `StorageContainerRequest` and `StorageContainerResponse`

Master Issue: #1205

Author: Sijie Guo <sijie@apache.org>

Reviewers: Enrico Olivelli <eolivelli@gmail.com>, Jia Zhai <None>

This closes #1452 from sijie/remove_storage_container_request_response
sijie added a commit that referenced this issue Jun 4, 2018
Descriptions of the changes in this PR:

*Motivation*

table service was built over bookkeeper ledgers. it is an extension to bookies' functionalities, so it is much convenient to start the service as one additional component with bookie, just like how we start `AutoRecovery` along with bookie.

*Solution*

- include `stream/server` module as part of bookkeeper server/all binary distributions
- abstract `StorageServer` to allow controlling whether to start bookie or not
- provide a ServerLifecycleComponent of `StorageServer`, so it can be used as an extra component of bookie

*Tests*

- improve the integration tests to start table service as part of bookie containers
- move `LocationClientTest` to container based integration tests

Master Issue: #1205

Author: Sijie Guo <sijie@apache.org>

Reviewers: Enrico Olivelli <eolivelli@gmail.com>, Jia Zhai <None>

This closes #1422 from sijie/start_table_service
sijie added a commit that referenced this issue Jun 4, 2018
…n` to `tests/integration/cluster`

Descriptions of the changes in this PR:

The original integration tests were written based a non-dockerized standalone stream cluster. Moved them to use
the dockerized integration test framework. So all the integration tests are actually testing the table service run as part of bookies.

This change is based on #1422 .
a371ff2 is the change in this PR to be reviewed.

Master Issue: #1205

Author: Sijie Guo <sijie@apache.org>

Reviewers: Jia Zhai <None>

This closes #1423 from sijie/move_more_stream_it_tests
sijie added a commit that referenced this issue Jun 4, 2018
Descriptions of the changes in this PR:

*Motivation*

Current almost every grpc requests are wrapped into `StorageContainerRequest` and their responses
are wrapped into `StorageContainerResponse`. It makes things a bit complicated on adding new grpc
services.

To simplify things, we can use grpc ClientInterceptor to stamp container information (e.g. scId)
into the request metadata and write a grpc service registry to take the container information from
request metadata and dispatch requests to containers.

In order to achieve it, we need to move the grpc services to storage container.

*Solution*

This PR moves grpc services from server module to storage module, so we can simplify the wire protocols.

Master Issue: #1205

Author: Sijie Guo <sijie@apache.org>

Reviewers: Enrico Olivelli <eolivelli@gmail.com>, Jia Zhai <None>

This closes #1428 from sijie/move_grpc_service_to_storage
sijie added a commit that referenced this issue Jun 4, 2018
Descriptions of the changes in this PR:

This is a subsequent change after #1428.

*Motivation*

Current almost every grpc requests are wrapped into `StorageContainerRequest` and their responses
are wrapped into `StorageContainerResponse`. It makes things a bit complicated on adding new grpc
services.

*Changes*

To simplify things, this PR introduces two functionalities for simplifying dispatching container requests/responses.

1) *StorageContainerClientInterceptor*: A grpc `ClientInterceptor` that stamps container information (currently is `scId`) into the requests' metadata before sending the requests to the wire.

2) A simple grpc reverse proxy to dispatch grpc requests to the channels provided by a `ChannelFinder`.

*Tests*

1. Existing unit tests covered client interceptor changes.
2. Introduced a `stream-storage-tests-common` module to include common classes that would be used for testing.
3. Introduced a `PingPongService` for testing reverse proxy : unary/client-streaming/server-streaming/bidi-streaming.

Master Issue: #1205

Author: Sijie Guo <sijie@apache.org>

Reviewers: Jia Zhai <None>

This closes #1430 from sijie/interceptor_container_requests
sijie added a commit that referenced this issue Jun 4, 2018
…uests

Descriptions of the changes in this PR:

*Motivation*

*Changes*

**Client Changes**

Changed `StorageContainerChannel` to add client interceptor to stamp storage container id into the the requests' metadata.

**Server Changes**

- moved the stream storage logic out of storage containers to a `service` package. the main logic will be kept in `RangeStoreService` and `RangeStoreServiceFactory`.
- make the storage container logic for hosting `StorageContainerService`.
- Each storage container will run an inprogress grpc server for serving the grpc services registered to the container and provide an `channel` for accessing those grpc service.
- Changed the server to use reverse proxy for serving/proxying storage container requests.

*NOTE*

This change doesn't directly remove `StorageContainerRequest` and `StorageContainerResponse`.  It would be done in a subsequent change.

Master Issue: #1205

Author: Sijie Guo <sijie@apache.org>

Reviewers: Jia Zhai <None>

This closes #1448 from sijie/storage_container_channel
sijie added a commit that referenced this issue Jun 4, 2018
…sponse

Descriptions of the changes in this PR:

*Motivation*

 #1448 uses reverse proxy for serving storage container requests and responses. so the `StorageContainerRequest` and `StorageContainerResponse`
 become redundant.

*Solution*

removes `StorageContainerRequest` and `StorageContainerResponse`

Master Issue: #1205

Author: Sijie Guo <sijie@apache.org>

Reviewers: Enrico Olivelli <eolivelli@gmail.com>, Jia Zhai <None>

This closes #1452 from sijie/remove_storage_container_request_response
@eolivelli
Copy link
Contributor

@sijie seems we are only lacking the 'documentation' subtask and this will be closed

@sijie
Copy link
Member Author

sijie commented Sep 6, 2018

@eolivelli yes the basic functions are there. so we can mark this BP as closed after we have documentation. there will be more improvements/changes in future, but they are separated from this BP.

@eolivelli eolivelli modified the milestones: 4.8.0, 4.9.0 Sep 6, 2018
@xiemeilong
Copy link

When will this be documented. It seems a useful feature.

@hsaputra
Copy link
Contributor

@sijie , @eolivelli - seems like the issue is still missing documentation for the Table service API

@eolivelli
Copy link
Contributor

@hsaputra you are totally right !

we really need doc for this feature

cc @jennifer88huang

@sijie
Copy link
Member Author

sijie commented Feb 3, 2021

@hsaputra Apologized that I missed this message! Yes, we still need documentation. The feature is still a beta version. I know your team at Splunk is actively working on it. I am not sure if there are any major API changes or not. We might want to wait for your team to contribute the changes back to the bookkeeper project before @jennifer88huang starts writing the documentation for it. What do you think?

@hsaputra
Copy link
Contributor

hsaputra commented Feb 9, 2021

HI @sijie , sorry for late response.
I think it would be ok to wait until the contribution is coming back upstream. It gives motivation to accelerate the initiative.

@hsaputra
Copy link
Contributor

hsaputra commented Feb 11, 2021

Hey @sijie , @eolivelli - After talking to team at Splunk working on the State Store upgrades, looks like there is no, or very little changes to the Table Service API themselves.

I have filed bug for adding documentation for this new feature. It should be ok for @jennifer88huang to start adding doc for this feature.

FYI @merlimat

@sijie
Copy link
Member Author

sijie commented Feb 11, 2021

@hsaputra thank you for the heads up! I will plan that with @jennifer88huang.

@HammadB
Copy link

HammadB commented Mar 16, 2023

Is there any plan to add documentation for how to use the table service?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/tableservice changes related to table service BP type/proposal
Projects
None yet
Development

No branches or pull requests

5 participants