[
https://issues.apache.org/jira/browse/YARN-4957?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15289393#comment-15289393
]
Carlo Curino commented on YARN-4957:
------------------------------------
[~seanpo03], please address the checkstyle/Javadoc issues... and remind us why
it is ok to commit with tests not passing (e.g., refer to respective jiras to
fix them).
Below a list of nits/mostly minor stuff (some of which is just taste or
questions):
Also I notice in the comments of the API we leak the current implementation by
saying "The ResourceManager responds with a new, monotonically increasing
ReservationId"... I would just say is unique. We might want later on to return
non monotonically increasing reservationIds, and definitely not have people
rely on this.
Please check comments for small typos/oddities (e.g., "@param request request
to get a new <code>ReservationId</code>", "to submit an reservation")
Are the classes "@Stable"? the methods are marked "@Unstable"... I am asking as
I am not sure how we should use stable/unstable at the class vs method level.
Why YARNClient.createReservation() in stead of
YARNClient.createNewReservationId()?
Just a question to make sure I follow correctly: The submitReservation is
idempotent if we submit exactly the same ReservationDefinition, but will reject
if I try to reuse the reservationId with same Id but a different
ReservationDefinition. If the user intend to modify prior submission it should
use update. The error message could be made more informative, but also stating
the ReservationDefinition is different (consider to update existing
reservationId or submit with a new reservation)
Are the comments and code of "ReservationSubmissionResponse" consistent?
In the "ReservationSubmissionResponse" should we retain the ReservationId? I
think might make the implementation on the client easier at times (if multiple
reservations are submitted asynchronsouly or something like that, it helps
retain the info of "who got accepted"). For now invocations are synch, but we
might want to change it later. Thoughts?
You have copy and pasted code in TestYarnClient and TestClientRMService,
consider (if possible for packaging etc..) to put this in a shared testbase
class.
What do "list" return if I have obtained a ReservationId, but never used it to
submit a reservation?
Why in the tests do you use a mock "generateReservationId(int)" instead of
using the createReservation API?
In documentation I would not talk to the reader directly (e.g., "With the New
Reservation API, you can obtain a reservation-id ") use impersonal forms if
possible (unless the rest of docs are written that way).
> Add getNewReservation in ApplicationClientProtocol
> --------------------------------------------------
>
> Key: YARN-4957
> URL: https://issues.apache.org/jira/browse/YARN-4957
> Project: Hadoop YARN
> Issue Type: Sub-task
> Components: applications, client, resourcemanager
> Affects Versions: 2.8.0
> Reporter: Subru Krishnan
> Assignee: Sean Po
> Labels: api-breaking
> Attachments: YARN-4957.v0.patch, YARN-4957.v1.patch,
> YARN-4957.v2.patch, YARN-4957.v3.patch, YARN-4957.v4.patch,
> YARN-4957.v5.1.patch, YARN-4957.v5.2.patch, YARN-4957.v5.patch
>
>
> Currently submitReservation returns a ReservationId if sucessful. This JIRA
> propose adding a getNewReservation in ApplicationClientProtocol for the
> following reasons:
> * Prevent zombie reservations in the face of client and/or network failures
> post submitReservation
> * Align reservation submission with application submission
--
This message was sent by Atlassian JIRA
(v6.3.4#6332)
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
