Skip to content

Commit

Permalink
Browse files Browse the repository at this point in the history
65741: kvserver: don't reset local keys in tscache on r1 lease move r=andreimatei a=andreimatei

applyReadSummaryToTimestampCache had a bug in how it dealt with r1's
descriptor: it bumped the timestamp cache over the whole keyspace that
the descriptor declared ([\x00,\x02)). That's bad, because it includes
the \x01... keys. That's the "local" keyspace, which doesn't belong to
any range in particular (so, r1's descriptor is arguably lying). Bumping
the timestamp cache over the local keyspace had the consequence (maybe
among others) that the txn tombstone keys were included, leading to
transactions being aborted when they came to create their txn record
because the bumped timestamp cache was saying that there might have been
a tombstone in there.

Release note: A bug causing transactions to be spuriously aborted in
rare circumstances has been fixed.

66132: Auto-create docs issues when PRs with valid release notes are merged r=yzdocs a=yzdocs

Fixes #59546

Implement a script run by a TeamCity build configuration that auto-creates an issue in the docs repo when a CRDB PR is merged, and that PR contains at least one commit with at least one valid release note that is not related to bug fix.

Release note: None

Dependent upon https://github.com/cockroachdb/dev-inf/issues/447

66165: kvclient: protect agaist DelRange with limit running as 1PC r=andreimatei a=andreimatei

Such a request is dangerous because it can be very expensive. Because of
how our "intent span" collection works, for requests from the
EndTransaction batch, the intents they've laid down are inferred from
the request, not from the response (since there's no opportunity to get
a response before the intents need to be attached to the ET).  This has
potentially catastrophic consequences for DeleteRanges with limits in
the ET batch - the limit is ignored for the purposes of tracking the
intents.  If the batch happens to also have a BeginTransaction, and if
we manage to execute it as a 1PC (so, the DistSender doesn't need to
split it and the store manages to execute it as 1PC), then I think we're
fine - there's no intents to speak of and the ET is ignored.  If it's
not executed as a 1PC, however, the cleanup process is going to scan the
whole key span specified in the DeleteRange, with no respect for the
limit.

This patch has the DistSender check for and reject such requests.

Touches #37457

Release note: None

Co-authored-by: Andrei Matei <[email protected]>
Co-authored-by: YZ Chin <[email protected]>
  • Loading branch information
3 people committed Jun 14, 2021
4 parents b8d1f11 + 2a2e749 + c0e49a9 + fc91c5d commit b60e941
Show file tree
Hide file tree
Showing 12 changed files with 304 additions and 334 deletions.
59 changes: 59 additions & 0 deletions build/teamcity-release-notes.sh
Original file line number Diff line number Diff line change
@@ -0,0 +1,59 @@
#!/usr/bin/env bash

remove_files_on_exit() {
rm -f commits.json match.json message messages.txt prnums valid
}
trap remove_files_on_exit EXIT

# A merge may contain multiple PRs due to craig(bot) activity
# Get PR numbers for current merge
curl -H 'Authorization: token $GITHUB_API_TOKEN' https://api.github.com/search/issues?q=sha:$BUILD_VCS_NUMBER+repo:cockroachdb/cockroach | jq .items[].number > prnums

# For each PR, process its commit(s)
process_pr_commits() {
# Get commits given PR number
commits=$(curl -H 'Authorization: token $GITHUB_API_TOKEN' https://api.github.com/repos/cockroachdb/cockroach/pulls/$1/commits)
commits="${commits//$'\''/'%27'}"
echo $commits > commits.json
pr_title=$(curl -H 'Authorization: token $GITHUB_API_TOKEN' https://api.github.com/repos/cockroachdb/cockroach/pulls/$1 | jq .title)
commitcount=$( jq length commits.json )
commitctr=0

# Iterate through all commit messages on this PR
while [ $commitctr -lt $commitcount ]
do
jq -r --arg i "$commitctr" '.[$i|tonumber].commit.message' commits.json > message
# For each commit message, check whether it contains at least one release note
bool=$( jq -r --arg i "$commitctr" '.[$i|tonumber].commit.message|test("[rR]elease [nN]ote")' commits.json )
# Extract the type(s) of release note(s)
jq -r --arg i "$commitctr" '.[$i|tonumber].commit.message|match("[rR]elease [nN]ote (?<type>.*): .*")' commits.json > match.json
# Check whether there is at least one release note type that is not none" or "bug"-related
jq -r '.captures | .[] | .string|test("[nN]one|[bB]ug")' match.json > valid

# If conditions fulfilled, format the release note(s) and create a docs issue
if $bool == true && grep -q false valid
then
echo "https://github.com/cockroachdb/cockroach/pull/$1" > messages.txt
echo "---" >> messages.txt
cat message | sed -n '/[rR]elease [nN]ote/,$p' >> messages.txt
body=$(cat messages.txt)
body="${body//'%'/'%25'}"
body="${body//$'\n'/' '}"
body="${body//$'\r'/' '}"
body="${body//$'\\'/'%5C'}"
echo $body
# Get merge branch
merge_branch=$(curl -H 'Authorization: token $GITHUB_API_TOKEN' https://api.github.com/repos/cockroachdb/cockroach/pulls/$1 | jq .base.ref)
curl -X POST -H 'Authorization: token $GITHUB_API_TOKEN' \
-d '{"title": "'"$pr_title"'", "labels": ["C-product-change", "'"$merge_branch"'"], "body": "'"$body"'"}' \
https://api.github.com/repos/cockroachdb/docs/issues
fi
commitctr=`expr $commitctr + 1`
done
}

# For each PR in the merge, process the PR's associated commits
cat prnums | while read prnum
do
process_pr_commits $prnum
done
11 changes: 4 additions & 7 deletions pkg/keys/constants.go
Original file line number Diff line number Diff line change
Expand Up @@ -22,9 +22,7 @@ import (
// These constants are single bytes for performance. They allow single-byte
// comparisons which are considerably faster than bytes.HasPrefix.
const (
LocalPrefixByte = '\x01'
localMaxByte = '\x02'
meta1PrefixByte = localMaxByte
meta1PrefixByte = roachpb.LocalMaxByte
meta2PrefixByte = '\x03'
metaMaxByte = '\x04'
systemPrefixByte = metaMaxByte
Expand All @@ -43,10 +41,9 @@ var (
MaxKey = roachpb.KeyMax

// LocalPrefix is the prefix for all local keys.
LocalPrefix = roachpb.Key{LocalPrefixByte}
// LocalMax is the end of the local key range. It is itself a global
// key.
LocalMax = roachpb.Key{localMaxByte}
LocalPrefix = roachpb.LocalPrefix
// LocalMax is the end of the local key range. It is itself a global key.
LocalMax = roachpb.LocalMax

// localSuffixLength specifies the length in bytes of all local
// key suffixes.
Expand Down
Loading

0 comments on commit b60e941

Please sign in to comment.