Re: Process for requesting a PR to be included in a RADOS run

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Hi Ilya,

We have had some main branch PRs tagged with core + needs-qa +
wip-yuri-testing just sit there for a while (e.g. [1], [2]).  In the
CLT meeting it became clear that there is some confusion about the
process: for example, Yuri doesn't pick up main branch PRs on his own
and instead expects a nod from someone on the RADOS team.  This is
because unlike with release branch PRs, the relative priority isn't
always clear.

Thanks for bringing these PRs to my attention. From this, I can see that the PRs have been included in a "wip-yuri-testing" batch and are awaiting a review from RADOS. I am the one who usually approves RADOS runs, however, I don't always have enough bandwidth to get through all of them very quickly. I have been thinking of enlisting a few other core team members to help reviews go quicker. However, in general, unless Yuri or I are made aware that a certain PR is very high priority, the ETA for a review can take 1-2 weeks. (3 weeks is a bit long though, so I will make sure that gets addressed).

Also, anyone can schedule a rados suite for their PRs at any time and do their own analysis if on a tight time schedule. Yuri's QA batches are offered to conserve resources and help people with their testing, but it's not the only solution to get a PR merged. I am happy to help anyone who wants to schedule their own rados suite and do their own analysis. Especially if a PR is, for instance, mainly CephFS and only touches one core file, it may only need some light testing in the rados suite.

On the other hand, not all PRs that get tagged with core by the labeler
necessarily need a RADOS run.  For example, src/mon/MDSMonitor.cc would
probably be better covered by the CephFS suite.

If you see any files encapsulated by the "core" label that don't belong, we can re-evaluate or adjust the label here: https://github.com/ceph/ceph/blob/main/.github/labeler.yml

What are the expectations from the RADOS team here?  Is core +
  needs-qa combination sufficient or something else is required?

This is the query that is used to batch main core PRs: https://github.com/ceph/ceph/pulls?q=is%3Apr+is%3Aopen+no%3Amilestone+label%3Aneeds-qa+review%3Aapproved+-label%3Aneeds-rebase+-label%3Astale+-label%3ADNM+label%3Acore
If a PR has "core" + "needs-qa" + is approved, it should land in this query (in addition to a few other things like no "needs-rebase" and no "DNM").

Since RADOS is kind of a catch-all suite, is there anyone sweeping
  PRs tagged with needs-qa but not core (e.g. common or build/ops) for
  including in RADOS runs?  Such PRs tend to linger for months (e.g.
  [3], [4]).  Perhaps we need to introduce a new needs-rados-qa label
  specifically for such cases?  If used on PRs that are anyway tagged
  with core it would make a request more explicit.

Good point! The query above does not touch build/ops PRs, so if we want these picked up in the rados suite, we should either adjust the label to include these files, or introduce a new label. I don't think we need to add a new label per-se, I'm thinking that it would be more appropriate to tell the labeler to add the core label to these files.
 

On Wed, Jul 12, 2023 at 11:52 AM Ilya Dryomov <idryomov@xxxxxxxxx> wrote:
Hello,

We have had some main branch PRs tagged with core + needs-qa +
wip-yuri-testing just sit there for a while (e.g. [1], [2]).  In the
CLT meeting it became clear that there is some confusion about the
process: for example, Yuri doesn't pick up main branch PRs on his own
and instead expects a nod from someone on the RADOS team.  This is
because unlike with release branch PRs, the relative priority isn't
always clear.

On the other hand, not all PRs that get tagged with core by the labeler
necessarily need a RADOS run.  For example, src/mon/MDSMonitor.cc would
probably be better covered by the CephFS suite.

Questions:

- What are the expectations from the RADOS team here?  Is core +
  needs-qa combination sufficient or something else is required?

- Since RADOS is kind of a catch-all suite, is there anyone sweeping
  PRs tagged with needs-qa but not core (e.g. common or build/ops) for
  including in RADOS runs?  Such PRs tend to linger for months (e.g.
  [3], [4]).  Perhaps we need to introduce a new needs-rados-qa label
  specifically for such cases?  If used on PRs that are anyway tagged
  with core it would make a request more explicit.

[1] https://github.com/ceph/ceph/pull/50503
[2] https://github.com/ceph/ceph/pull/52124
[3] https://github.com/ceph/ceph/pull/48672
[4] https://github.com/ceph/ceph/pull/50301

Thanks,

                Ilya



--

Laura Flores

She/Her/Hers

Software Engineer, Ceph Storage

Chicago, IL

lflores@xxxxxxx | lflores@xxxxxxxxxx
M: +17087388804



_______________________________________________
Dev mailing list -- dev@xxxxxxx
To unsubscribe send an email to dev-leave@xxxxxxx

[Index of Archives]     [CEPH Users]     [Ceph Devel]     [Ceph Large]     [Information on CEPH]     [Linux BTRFS]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux