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

Rest API for Ledger Offloading #1639

Merged
merged 7 commits into from
May 2, 2018
Merged

Conversation

ivankelly
Copy link
Contributor

@ivankelly ivankelly commented Apr 25, 2018

Implemented for both V1 and V2 topic name formats. Each persistent
topic has a /offload endpoint. A PUT to this endpoint will trigger offloading.
This put takes a message ID as parameter, which is the highest message
ID that will be offloaded. If there is already an offload in progress, the put
will return a HTTP 409 response, signifying that there is already an offload
in progress. A GET on the endpoint will return the status of a previous
triggered offload, if any.

This patch also adds basic support for setting the Offloader
implementation in the broker (needed for testing). Subsequent patches
will make this configurable through ServiceConfiguration.

Master Issue: #1511

Implemented for both V1 and V2 topic name formats. API takes a message
ID, up to which the broker will try to offload messages. It returns
the message ID of the first message in the topic which has not been
offloaded.

This patch also adds basic support for setting the Offloader
implementation in the broker (needed for testing). Subsequent patches
will make this configurable through ServiceConfiguration.
@ivankelly
Copy link
Contributor Author

retest this please // integration test timeout

Copy link
Member

@sijie sijie left a comment

Choose a reason for hiding this comment

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

@ivankelly : what would be the expected behavior when triggering offloading?

if you are issuing an admin operation, are you blocking on waiting the result? from the implementation, it seems to be that behavior.

I think the behavior is not that useful. I would expect users would like a behavior like:

  • users issue trigger an offload rest request. if broker is already is offloading, return "offloading inprogress"; if broker is not offloading, scheduling an offload, return "offloading scheduled/inprogress" and return that last message id as some sort of offload-task-id. so later on that the user can use offload-task-id (message id) to query if the offload is completed.

@ivankelly
Copy link
Contributor Author

@sijie ya, I thought about doing it like that when I was finishing this version. Compaction is done like that. Will change this.

@sijie
Copy link
Member

sijie commented Apr 25, 2018

cool thanks @ivankelly

@ivankelly
Copy link
Contributor Author

retest this please // PersistentFailoverE2ETest.testSimpleConsumerEventsWithoutPartition

@merlimat merlimat added this to the 2.1.0-incubating milestone May 1, 2018
@merlimat merlimat added the type/feature The PR added a new feature or issue requested a new feature label May 1, 2018
@ivankelly
Copy link
Contributor Author

@sijie pinging. This is ready for review again.

@sijie
Copy link
Member

sijie commented May 2, 2018

@ivankelly yeah. will review. I was holding it after Matteo cut the 2.0 branch.

@sijie sijie merged commit 5f678e0 into apache:master May 2, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/tieredstorage type/feature The PR added a new feature or issue requested a new feature
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants