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(a)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(a)ibm.com | lflores(a)redhat.com <lflores(a)redhat.com>
M: +17087388804