linter: Add PR check to encourage including issue or epic refs in the PR/commit messages #77376
Assignees
Labels
C-enhancement
Solution expected to add code/behavior + preserve backward-compat (pg compat issues are exception)
T-dev-inf
Comments
jlinder
added
C-enhancement
angles-n-daemons
added a commit
to angles-n-daemons/cockroach
that referenced
this issue
Sep 4, 2024
Fixes: cockroachdb#77376 Release note (ops change): --- security: add ttl metrics for certificate expiration Currently, cockroach only exposes point in time certificate expiration metrics. If the certificate is to expire 1 day from now, we expose a gauge `security.certificate.expiration.<cert-type>` which is the unix timestamp when it will expire. This PR also exposes a ttl metric `security.certificate.ttl.<cert-type>` so that consumers of this information can run operations based on their distance to certificate expiration without additional transformations. Additionally, this PR refactors how the expiration gauges are set, so that reads of the gauge directly reference the value of the certificate.
angles-n-daemons
added a commit
to angles-n-daemons/cockroach
that referenced
this issue
Sep 6, 2024
Fixes: cockroachdb#77376 Release note (ops change): --- security: add ttl metrics for certificate expiration Currently, cockroach only exposes point in time certificate expiration metrics. If the certificate is to expire 1 day from now, we expose a gauge `security.certificate.expiration.<cert-type>` which is the unix timestamp when it will expire. This PR also exposes a ttl metric `security.certificate.ttl.<cert-type>` so that consumers of this information can run operations based on their distance to certificate expiration without additional transformations. Additionally, this PR refactors how the expiration gauges are set, so that reads of the gauge directly reference the value of the certificate.
angles-n-daemons
added a commit
to angles-n-daemons/cockroach
that referenced
this issue
Sep 10, 2024
Currently, cockroach only exposes point in time certificate expiration metrics. If the certificate is to expire 1 day from now, we expose a gauge `security.certificate.expiration.<cert-type>` which is the unix timestamp when it will expire. This PR also exposes a ttl metric `security.certificate.ttl.<cert-type>` so that consumers of this information can run operations based on their distance to certificate expiration without additional transformations. Additionally, this PR refactors how the expiration gauges are set, so that reads of the gauge directly reference the value of the certificate. Epic: CRDB-40209 Fixes: cockroachdb#77376 Release note (ops change): new metrics which expose the ttl for various certificates
angles-n-daemons
added a commit
to angles-n-daemons/cockroach
that referenced
this issue
Sep 12, 2024
Currently, cockroach only exposes point in time certificate expiration metrics. If the certificate is to expire 1 day from now, we expose a gauge `security.certificate.expiration.<cert-type>` which is the unix timestamp when it will expire. This PR also exposes a ttl metric `security.certificate.ttl.<cert-type>` so that consumers of this information can run operations based on their distance to certificate expiration without additional transformations. Additionally, this PR refactors how the expiration gauges are set, so that reads of the gauge directly reference the value of the certificate. Epic: CRDB-40209 Fixes: cockroachdb#77376 Release note (ops change): new metrics which expose the ttl for various certificates
angles-n-daemons
added a commit
to angles-n-daemons/cockroach
that referenced
this issue
Sep 12, 2024
Currently, cockroach only exposes point in time certificate expiration metrics. If the certificate is to expire 1 day from now, we expose a gauge `security.certificate.expiration.<cert-type>` which is the unix timestamp when it will expire. This PR also exposes a ttl metric `security.certificate.ttl.<cert-type>` so that consumers of this information can run operations based on their distance to certificate expiration without additional transformations. Additionally, this PR refactors how the expiration gauges are set, so that reads of the gauge directly reference the value of the certificate. Epic: CRDB-40209 Fixes: cockroachdb#77376 Release note (ops change): new metrics which expose the ttl for various certificates
craig bot
pushed a commit
that referenced
this issue
Sep 13, 2024
130110: ops: add ttl metrics for certificate expiration r=angles-n-daemons a=angles-n-daemons ops: add ttl metrics for certificate expiration Currently, cockroach only exposes point in time certificate expiration metrics. If the certificate is to expire 1 day from now, we expose a gauge `security.certificate.expiration.<cert-type>` which is the unix timestamp when it will expire. This PR also exposes a ttl metric `security.certificate.ttl.<cert-type>` so that consumers of this information can run operations based on their distance to certificate expiration without additional transformations. Additionally, this PR refactors how the expiration gauges are set, so that reads of the gauge directly reference the value of the certificate. Epic: CRDB-40209 Fixes: #77376 Release note (ops change): new metrics which expose the ttl for various certificates 130699: replica_rac2: remove a todo and add another one r=pav-kv a=sumeerbhola The first todo related to Processor.OnDescChangedLocked is obsolete, since we enqueue for ready processing. But ready procesing is not unconditional, hence the new todo. Epic: CRDB-37515 Release note: None Co-authored-by: angles-n-daemons <[email protected]> Co-authored-by: sumeerbhola <[email protected]>
vidit-bhat
added a commit
to vidit-bhat/cockroach
that referenced
this issue
Oct 30, 2024
This PR adds the yaml configurations for the scale test PCR and Restore clusters Fixes: cockroachdb#77376 Release note: None
vidit-bhat
added a commit
to vidit-bhat/cockroach
that referenced
this issue
Oct 30, 2024
This PR adds the yaml configurations for the scale test PCR and Restore clusters Fixes: cockroachdb#77376 Release note: None
craig bot
pushed a commit
that referenced
this issue
Oct 30, 2024
133796: drtprod: add create and destroy yamls for pcr and restore clusters r=nameisbhaskar a=vidit-bhat This PR adds the yaml configurations for the scale test PCR and Restore clusters Fixes: #77376 Release note: None 133850: sql/catalog: fix panic inside sequence expression parsing r=fqazi a=fqazi Previously, if the correct overloads were not found for sequence builtins it was possible for the server to panic. This could happen when rewriting a CREATE TABLE expression with an invalid sequence builtin call. To address this, this patch updates the sequence logic to return the error instead of panicking on it. Fixes: #133399 Release note (bug fix): Address a panic inside CREATE TABLE AS if sequence builtin expressions had invalid function overloads. Co-authored-by: Vidit Bhat <[email protected]> Co-authored-by: Faizan Qazi <[email protected]>
miraradeva
added a commit
to miraradeva/cockroach
that referenced
this issue
Nov 14, 2024
In rare cases (e.g. cockroachdb#77376), two transactions can get repeatedly deadlocked while trying to write to same key(s): one aborts the other, but before it can proceed, the other transaction has restarted and acquired a lock on the key again. This can result in the max transaction retries being exceeded without either transaction succeeding. This commit adds a backoff to the transaction retry loop in `db.Txn`, which will hopefully help one transaction slow down and let the other one commit. Fixes: cockroachdb#77376 Release note: None
miraradeva
added a commit
to miraradeva/cockroach
that referenced
this issue
Nov 14, 2024
In rare cases (e.g. cockroachdb#77376), two transactions can get repeatedly deadlocked while trying to write to same key(s): one aborts the other, but before it can proceed, the other transaction has restarted and acquired a lock on the key again. This can result in the max transaction retries being exceeded without either transaction succeeding. This commit adds a backoff to the transaction retry loop in `db.Txn`, which will hopefully help one transaction slow down and let the other one commit. Fixes: cockroachdb#77376 Release note: None
miraradeva
added a commit
to miraradeva/cockroach
that referenced
this issue
Nov 14, 2024
In rare cases (e.g. cockroachdb#77376), two transactions can get repeatedly deadlocked while trying to write to same key(s): one aborts the other, but before it can proceed, the other transaction has restarted and acquired a lock on the key again. This can result in the max transaction retries being exceeded without either transaction succeeding. This commit adds a backoff to the transaction retry loop in `db.Txn`, which will hopefully help one transaction slow down and let the other one commit. Fixes: cockroachdb#77376 Release note: None
miraradeva
added a commit
to miraradeva/cockroach
that referenced
this issue
Nov 15, 2024
In rare cases (e.g. cockroachdb#77376), two transactions can get repeatedly deadlocked while trying to write to same key(s): one aborts the other, but before it can proceed, the other transaction has restarted and acquired a lock on the key again. This can result in the max transaction retries being exceeded without either transaction succeeding. This commit adds a backoff to the transaction retry loop in `db.Txn`, which will hopefully help one transaction slow down and let the other one commit. Fixes: cockroachdb#77376 Release note: None
craig bot
pushed a commit
that referenced
this issue
Nov 18, 2024
135173: kv: add a backoff to the retry loop in db.Txn r=miraradeva a=miraradeva In rare cases (e.g. #77376), two transactions can get repeatedly deadlocked while trying to write to same key(s): one aborts the other, but before it can proceed, the other transaction has restarted and acquired a lock on the key again. This can result in the max transaction retries being exceeded without either transaction succeeding. This commit adds a backoff to the transaction retry loop in `db.Txn`, which will hopefully help one transaction slow down and let the other one commit. Fixes: #77376 Release note: None 135253: roachtest: get exec traces in gossip/chaos/nodes=9 r=tbg a=tbg This test is known for strange behavior (#134527). A recent test failure[^1] shows that we "lose time" in more unexpected locations. I'm assuming an execution trace (of the roachtest process) will be helpful here, as we'll be able to determine whether the time is spent in roachtest or on the CockroachDB server side. [^1]: #135154 Epic: none Release note: None 135333: roachtest: fix "context cancelled" errors in db-console tests r=kyle-a-wong a=kyle-a-wong changes the context used when writing cypress artifacts to the test artifact directory. This is needed because the existing context is getting cancelled, pressumable when `rtCluster.RunE(ctx...` fails. Adds `-e NO_COLOR=1` to the docker run command so that the output is more humanreadable in log files Updates the tests to use `registry.StandardCockroach`. By default, `registry.RandomizedCockroach` is used, and `registry.RuntimeAssertionsCockroach` is built using `cockroach-short`, which does not include db-console in the binary. Resolves: #134808 Epic: none Release note: None Co-authored-by: Mira Radeva <[email protected]> Co-authored-by: Tobias Grieger <[email protected]> Co-authored-by: Kyle Wong <[email protected]>
miraradeva
added a commit
to miraradeva/cockroach
that referenced
this issue
Nov 19, 2024
In rare cases (e.g. cockroachdb#77376), two transactions can get repeatedly deadlocked while trying to write to same key(s): one aborts the other, but before it can proceed, the other transaction has restarted and acquired a lock on the key again. This can result in the max transaction retries being exceeded without either transaction succeeding. This commit adds a backoff to the transaction retry loop in `db.Txn`, which will hopefully help one transaction slow down and let the other one commit. Fixes: cockroachdb#77376 Release note: None
miraradeva
added a commit
to miraradeva/cockroach
that referenced
this issue
Dec 10, 2024
In cockroachdb#135173, we tried introducing a backoff in the transaction retry mechanism (in `db.Txn`) in order to help with large transactions that repeatedly get in a deadlock (as seen in cockroachdb#133431). This wasn't enough as we continue to see this type of failure in cockroachdb#136266. This commit increases the kvnemesis-specific retry backoff, which is meant to prevent thrashing of aborted transactions. We do see transactions getting aborted as part of deadlock resolution in both of the failures mentioned above, so hopefully this will help. Both failures above are rare and hard to repro. Fixes: cockroachdb#77376 Release note: None
angles-n-daemons
added a commit
to angles-n-daemons/cockroach
that referenced
this issue
Jan 8, 2025
Currently, cockroach only exposes point in time certificate expiration metrics. If the certificate is to expire 1 day from now, we expose a gauge `security.certificate.expiration.<cert-type>` which is the unix timestamp when it will expire. This PR also exposes a ttl metric `security.certificate.ttl.<cert-type>` so that consumers of this information can run operations based on their distance to certificate expiration without additional transformations. Additionally, this PR refactors how the expiration gauges are set, so that reads of the gauge directly reference the value of the certificate. Epic: CRDB-40209 Fixes: cockroachdb#77376 Release note (ops change): new metrics which expose the ttl for various certificates
hakuuww
added a commit
to hakuuww/cockroach
that referenced
this issue
Jan 28, 2025
When the raft leader wants to commit an entry(advance commit index), it must check the term of the entry it wants to commit and ensure the term number is the same as the current leader term(term of itself). Previously, this process may involve a storage access for the term number if the entry has been persisted on stable storage and no longer in the unstable log of the leader node. In fact this above scenario happens quite often. Which makes an optimization improving leader commit latency fruitful. This PR introduces an optimization that utilizes an invariant that is logically equivalent to checking whether the want-to-be-committed entry has the same term as the leader doing the commit. This is done by keeping an entry index in leader node's memory that signifies the point in time where the current leader became leader. When the leader wants to commit an entry, the leader compares the want-to-be-committed entry's index(available in memory) with the newly added index in this PR to determine whether the term matches. Thus completely eliminating storage access on this code path. A corresponding unit test is also modified to account for the elimination of storage access when leader tries to commit. Jira issue: CRDB-45763 Fixes: cockroachdb#137826 Release note (performance improvement): improved the internal raft commit logic. This slightly reduces the latency impact of a new client write request if a raft leader crash occured after the write request was submitted Fixes: cockroachdb#77376
hakuuww
added a commit
to hakuuww/cockroach
that referenced
this issue
Jan 29, 2025
When the raft leader wants to commit an entry(advance commit index), it must check the term of the entry it wants to commit and ensure the term number is the same as the current leader term(term of itself). Previously, this process may involve a storage access for the term number if the entry has been persisted on stable storage and no longer in the unstable log of the leader node. In fact this above scenario happens quite often. Which makes an optimization improving leader commit latency fruitful. This PR introduces an optimization that utilizes an invariant that is logically equivalent to checking whether the want-to-be-committed entry has the same term as the leader doing the commit. This is done by keeping an entry index in leader node's memory that signifies the point in time where the current leader became leader. When the leader wants to commit an entry, the leader compares the want-to-be-committed entry's index(available in memory) with the newly added index in this PR to determine whether the term matches. Thus completely eliminating storage access on this code path. A corresponding unit test is also modified to account for the elimination of storage access when leader tries to commit. Jira issue: CRDB-45763 Fixes: cockroachdb#137826 Release note (performance improvement): improved the internal raft commit logic. This slightly reduces the latency impact of a new client write request if a raft leader crash occured after the write request was submitted Fixes: cockroachdb#77376
xinhaoz
added a commit
to xinhaoz/cockroach
that referenced
this issue
Feb 10, 2025
… container Previously, each stats collector for a sql session would buffer a transaciton's stmt stats in a ssmemstorage.Container struct which has a lot of additional overhead when reading and writing as it manages synchronization logic. For this per-session writing, this writing is synchronous so there's no need to use such a container. We should just buffer the stmts in a simple slice. Note that this is simply a temporary simplified state. Eventually as part of cockroachdb#141024 we will move the writing to the shared container to be async from query execution. Epic: none Fixes: cockroachdb#77376 Release note: None
xinhaoz
added a commit
to xinhaoz/cockroach
that referenced
this issue
Feb 10, 2025
… container Previously, each stats collector for a sql session would buffer a transaciton's stmt stats in a ssmemstorage.Container struct which has a lot of additional overhead when reading and writing as it manages synchronization logic. For this per-session writing, this writing is synchronous so there's no need to use such a container. We should just buffer the stmts in a simple slice. Note that this is simply a temporary simplified state. Eventually as part of cockroachdb#141024 we will move the writing to the shared container to be async from query execution. Epic: none Fixes: cockroachdb#77376 Release note: None
xinhaoz
added a commit
to xinhaoz/cockroach
that referenced
this issue
Feb 11, 2025
… container Previously, each stats collector for a sql session would buffer a transaciton's stmt stats in a ssmemstorage.Container struct which has a lot of additional overhead when reading and writing as it manages synchronization logic. For this per-session writing, this writing is synchronous so there's no need to use such a container. We should just buffer the stmts in a simple slice. Note that this is simply a temporary simplified state. Eventually as part of cockroachdb#141024 we will move the writing to the shared container to be async from query execution. Epic: none Fixes: cockroachdb#77376 Release note: None
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Describe the solution you'd like
This new linter should be a program run by a GitHub action on every PR for these events: create, edit, reopen, synchronize.
What it will do:
For all PRs containing commits with Release note texts, other than bug fixes, Engineers can help establish the link to Jira epics in various ways:
Given Engineering’s practice to link GitHub issues to epics, referencing such an issue—as opposed to the epic it links to—is preferred in cases where both a Jira epic and GitHub issue exist. This makes information in the issue easily accessible to human readers while offering an epic link that is available via the issue to automation scripts.
The “Epic: None” option accounts for the fact that a significant amount of product changes occur spontaneously (serendipitously) as a by-product of investigations on main roadmap items. When this happens, Engineers take the initiative to make user-facing changes without prior processing by a PM, and thus without prior existence of a GH or Jira issue. We do not wish to reduce the ability of Engineers to innovate and improve our product in this way.
This solution will be encouraged, but not required, by a new optional blathers check on PRs. It will fail and give feedback on how to adjust the PR for the following cases:
Describe alternatives you've considered
This linter can't run in TeamCity because it needs to run on PR edit and reopen events. We considered using Blathers but decided to keep this in the cockroach repo.
Jira issue: CRDB-13558
Epic DEVINF-261
The text was updated successfully, but these errors were encountered: