Christophe Poucet, Patrick Hiesel, Luca Milanesio, Saša Živkov
January, 2024
Gerrit has not been great in choosing defaults; some of them resulted being unsafe (see 1 and 2).
Saša and Luca proposed to define what “unsafe” means in the Gerrit world for allowing the categorisation of configuration option settings; the proposed definition is “a parameter that, despite providing some benefits may not be suitable for all setups, risking performance issues or data corruption”.
Patrick pointed out that when introducing new settings or improvements to existing ones, a rule of thumb is to keep existing well-tested behaviour as default.
How to enforce the adoption of safe default? Luca took the action of amending the release process and including the cross-checking of new settings and their defaults by diff-ing what changed in the /Documentation
folder. The definition of “unsafe” default is proposed as amendment of the current contributors' guide.
Change 390074 broke the Gerrit setup of Chromium, because of the lack of visibility from the contributor of the impact of its changes on the closed-source fork of Gerrit maintained by Google.
Patrick pointed out that a Google Prober validates all incoming changes test in the internal CI/CD pipeline; however, the static resources servlet was not properly covered, and the breakage passed unnoticed to the live rollout.
Annotating the classes that are not used at Google with a @NotUsedAtGoogle
would not be enough to prevent future breakages because of the way that Google manages its customisation of the unused classes.
Google will continue patrolling the incoming changes on master and complete its internal probes suite for making sure that future modifications by the community would not impact the stability of its setups.
Gerrit has no way to enforce the uniqueness of some secondary IDs for a change, e.g. the triplet Project~Branch~Change-Id
, because of the need of trade-off uniqueness with the speed of execution.
Issue 313935024 has highlighted how it is quite simple to generate duplicates by uploading new patch sets on the same change before the index is updated. Gerrit index is a best-effort lookup, and it may be stale or out of sync with the underlying changes on the filesystem. Saša mentioned that the change reindexing operation can be a lengthy operation, depending on the size of the diff and the execution of complex Prolog rules: the scenario of duplicates caused by a stale index are happening in real production environments, Luca also confirmed the same.
Chris pointed out that it is impossible to assure more than one unique id without having a transactional store, which is in two-phase-commit with the Git repository. Hence, Gerrit needs to live with the problem and rely on the only unique identifier that the repository can assure: Project~ChangeNum
.
Luca will amend the Gerrit documentation, mentioning making sure that it is transparent for users and administrators that Project~ChangeNum
is the only identifier guaranteed to be unique, whilst the others are best effort and can produce duplicates.
After having declared the use of Prolog rules in repositories as deprecated, the Gerrit community needs to know when precisely the existing rules will end being supported.
Saša pointed out that there are still some corner cases where Gerrit submit requirements aren't enough for implementing the equivalent of some Prolog rules, for instance, when expressing a rule for changes having a set of files excluding a regex.
Patrick suggested working on extending the submit requirements predicates for covering the missing use cases.