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%... 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@gmail.com> 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 <https://ceph.io> Chicago, IL lflores@ibm.com | lflores@redhat.com <lflores@redhat.com> M: +17087388804