Skip to content

Commit

Permalink
Browse files Browse the repository at this point in the history
49135: kvserver: don't use marshaled proto for liveness CPut r=andreimatei a=andreimatei

Fixes #38308

Updates to the liveness records are done as CPuts. Before this patch,
the CPuts' expected value was the re-marshaled proto with the previous
version. As #38308 explains, that's a bad practice since it prevents the
proto's encoding to change in any way (e.g. fields can't be removed).
Instead, this patch moves to keeping track of the raw bytes that have
been read from the DB, besides the unmarshalled liveness protos. The raw
bytes are used as the expected values.

Release note: None

50658: geopb: rename Shape to ShapeType r=sumeerbhola a=otan

Rename Shape to ShapeType, in preparation for a new Shape protobuf.

Release note: None

50671: build: upgrade to Go 1.14 r=petermattis a=otan

Minor fixes required:
* Upgrade go.mod to prevent build.Import errors.
* Updated instructions to upgrade Go.
* Updated .pb.go files - these are tied with the gofmt of the
  environment and we cannot fix this.
* Update storage use cast DBSlice to DBString or it fails S1016 of
  staticcheck.
* Make lint fail if the import directory is not found.
* Fix acceptance test to include GOROOT or else build.Import fails from
  within the acceptance tests. The acceptance test is weirdly set up to
  run a Go binary from the outside container.

Checklist:
* [x] Adjust version in Docker image ([source](./builder/Dockerfile#L199-L200)).
* [x] Rebuild the Docker image and bump the `version` in `builder.sh` accordingly ([source](./builder.sh#L6)).
* [x] Bump the version in `go-version-check.sh` ([source](./go-version-check.sh)), unless bumping to a new patch release.
* [x] Bump the default installed version of Go in `bootstrap-debian.sh` ([source](./bootstrap/bootstrap-debian.sh#L40-42)).
* [ ] Update the `builder.dockerImage` parameter in the TeamCity [`Cockroach`](https://teamcity.cockroachdb.com/admin/editProject.html?projectId=Cockroach&tab=projectParams) and [`Internal`](https://teamcity.cockroachdb.com/admin/editProject.html?projectId=Internal&tab=projectParams) projects.

(I will do the last step after this has merged)

Resolves #48737.

Release note (general change): Bump the Go version to 1.14.



50675: httputil: remove req_go112.go r=knz a=otan

Release note: None

Co-authored-by: Andrei Matei <[email protected]>
Co-authored-by: Oliver Tan <[email protected]>
  • Loading branch information
3 people committed Jun 29, 2020
5 parents 195e95c + 9c0dce9 + 9c7560e + fffa545 + 9450e29 commit 74d7021
Show file tree
Hide file tree
Showing 51 changed files with 648 additions and 655 deletions.
12 changes: 8 additions & 4 deletions build/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -66,10 +66,14 @@ which may or may not work (and are not officially supported).
Please copy this checklist (based on [Basic Process](#basic-process)) into the relevant commit message, with a link
back to this document and perform these steps:

* [ ] Adjust the Pebble tests to run in 1.14.
* [ ] Adjust version in Docker image ([source](./builder/Dockerfile#L199-L200)).
* [ ] Rebuild the Docker image and bump the `version` in `builder.sh` accordingly ([source](./builder.sh#L6)).
* [ ] Rebuild and push the Docker image (following [Basic Process](#basic-process))
* [ ] Bump the version in `builder.sh` accordingly ([source](./builder.sh#L6)).
* [ ] Bump the version in `go-version-check.sh` ([source](./go-version-check.sh)), unless bumping to a new patch release.
* [ ] Bump the go version in `go.mod`. You may also need to rerun `make vendor_rebuild` if vendoring has changed.
* [ ] Bump the default installed version of Go in `bootstrap-debian.sh` ([source](./bootstrap/bootstrap-debian.sh#L40-42)).
* [ ] Replace other mentions of the older version of go (grep for `golang:<old_version>` and `go<old_version>`).
* [ ] Update the `builder.dockerImage` parameter in the TeamCity [`Cockroach`](https://teamcity.cockroachdb.com/admin/editProject.html?projectId=Cockroach&tab=projectParams) and [`Internal`](https://teamcity.cockroachdb.com/admin/editProject.html?projectId=Internal&tab=projectParams) projects.

You can test the new builder image in TeamCity by using the custom parameters
Expand All @@ -94,14 +98,14 @@ When updating a dependency, you should run `go mod tidy` after `go get` to ensur
are removed from go.sum.

You must then run `make vendor_rebuild` to ensure the modules are installed. These changes must
then be committed in the submodule directory (see Working with Submodules).
then be committed in the submodule directory (see [Working with Submodules](#working-with-submodules)).

Programs can then be run using `go build -mod=vendor ...` or `go test -mod=vendor ...`.

### Removing a dependency

When a dependency has been removed, run `go mod tidy` and then `make vendor_rebuild`. Then follow
the Working with Submodules steps below.
When a dependency has been removed, run `go mod tidy` and then `make vendor_rebuild`.
Then follow the [Working with Submodules](#working-with-submodules) steps.

### Requiring a new tool

Expand Down
4 changes: 2 additions & 2 deletions build/bootstrap/bootstrap-debian.sh
Original file line number Diff line number Diff line change
Expand Up @@ -46,9 +46,9 @@ sudo tar -C /usr -zxf /tmp/cmake.tgz && rm /tmp/cmake.tgz

# Install Go.
trap 'rm -f /tmp/go.tgz' EXIT
curl -fsSL https://dl.google.com/go/go1.13.9.linux-amd64.tar.gz > /tmp/go.tgz
curl -fsSL https://dl.google.com/go/go1.14.4.linux-amd64.tar.gz > /tmp/go.tgz
sha256sum -c - <<EOF
f4ad8180dd0aaf7d7cda7e2b0a2bf27e84131320896d376549a7d849ecf237d7 /tmp/go.tgz
7011af3bbc2ac108d1b82ea8abb87b2e63f78844f0259be20cde4d42c5c40584 /tmp/go.tgz
EOF
sudo tar -C /usr/local -zxf /tmp/go.tgz && rm /tmp/go.tgz

Expand Down
2 changes: 1 addition & 1 deletion build/builder.sh
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@
set -euo pipefail

image=cockroachdb/builder
version=20200421-180956
version=20200625-145628

function init() {
docker build --tag="${image}" "$(dirname "${0}")/builder"
Expand Down
4 changes: 2 additions & 2 deletions build/builder/Dockerfile
Original file line number Diff line number Diff line change
Expand Up @@ -197,8 +197,8 @@ RUN curl -fsSL https://github.com/Kitware/CMake/releases/download/v3.17.0/cmake-
# releases of Go will no longer be run in CI once it is changed. Consider
# bumping the minimum allowed version of Go in /build/go-version-chech.sh.
RUN apt-get install -y --no-install-recommends golang \
&& curl -fsSL https://storage.googleapis.com/golang/go1.13.9.src.tar.gz -o golang.tar.gz \
&& echo '34bb19d806e0bc4ad8f508ae24bade5e9fedfa53d09be63b488a9314d2d4f31d golang.tar.gz' | sha256sum -c - \
&& curl -fsSL https://storage.googleapis.com/golang/go1.14.4.src.tar.gz -o golang.tar.gz \
&& echo '7011af3bbc2ac108d1b82ea8abb87b2e63f78844f0259be20cde4d42c5c40584 golang.tar.gz' | sha256sum -c - \
&& tar -C /usr/local -xzf golang.tar.gz \
&& rm golang.tar.gz \
&& cd /usr/local/go/src \
Expand Down
3 changes: 1 addition & 2 deletions build/go-version-check.sh
Original file line number Diff line number Diff line change
Expand Up @@ -6,8 +6,7 @@
# To bump the required version of Go, edit the appropriate variables:

required_version_major=1
minimum_version_minor=13
minimum_version_13_patch=4
minimum_version_minor=14

go=${1-go}

Expand Down
2 changes: 1 addition & 1 deletion build/teamcity-acceptance.sh
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,6 @@ run cd pkg/acceptance
ln -s ../../artifacts artifacts
# NB: json has to be enabled when building the test binary,
# which makes this harder to get right than is worth it.
run_text_test github.com/cockroachdb/cockroach/pkg/acceptance env TZ=America/New_York stdbuf -eL -oL ./acceptance.test -l "$TMPDIR" -test.v -test.timeout 30m
run_text_test github.com/cockroachdb/cockroach/pkg/acceptance env TZ=America/New_York GOROOT=/home/agent/work/.go stdbuf -eL -oL ./acceptance.test -l "$TMPDIR" -test.v -test.timeout 30m
run cd ../..
tc_end_block "Run acceptance tests"
2 changes: 1 addition & 1 deletion build/teamcity-verify-archive.sh
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ run docker run \
--rm \
--volume="$(cd "$(dirname "$0")" && pwd):/work:ro" \
--workdir="/work" \
golang:1.13-buster ./verify-archive.sh
golang:1.14-buster ./verify-archive.sh
tc_end_block "Test archive"

tc_start_block "Clean up"
Expand Down
10 changes: 5 additions & 5 deletions docs/generated/sql/bnf/stmt_block.bnf
Original file line number Diff line number Diff line change
Expand Up @@ -2278,10 +2278,10 @@ const_datetime ::=
const_geo ::=
'GEOGRAPHY'
| 'GEOMETRY'
| 'GEOMETRY' '(' geo_shape ')'
| 'GEOGRAPHY' '(' geo_shape ')'
| 'GEOMETRY' '(' geo_shape ',' signed_iconst ')'
| 'GEOGRAPHY' '(' geo_shape ',' signed_iconst ')'
| 'GEOMETRY' '(' geo_shape_type ')'
| 'GEOGRAPHY' '(' geo_shape_type ')'
| 'GEOMETRY' '(' geo_shape_type ',' signed_iconst ')'
| 'GEOGRAPHY' '(' geo_shape_type ',' signed_iconst ')'

opt_varying ::=
'VARYING'
Expand Down Expand Up @@ -2467,7 +2467,7 @@ opt_timezone ::=
| 'WITHOUT' 'TIME' 'ZONE'
|

geo_shape ::=
geo_shape_type ::=
'POINT'
| 'LINESTRING'
| 'POLYGON'
Expand Down
2 changes: 1 addition & 1 deletion go.mod
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
module github.com/cockroachdb/cockroach

go 1.13
go 1.14

require (
cloud.google.com/go v0.34.0
Expand Down
2 changes: 1 addition & 1 deletion pkg/acceptance/compose/gss/psql/Dockerfile
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
# Build the test binary in a multistage build.
FROM golang:1.13 AS builder
FROM golang:1.14 AS builder
WORKDIR /workspace
COPY . .
RUN go get -d -t -tags gss_compose
Expand Down
2 changes: 1 addition & 1 deletion pkg/ccl/storageccl/engineccl/rocksdb.go
Original file line number Diff line number Diff line change
Expand Up @@ -130,7 +130,7 @@ func cStringToGoString(s C.DBString) string {
return ""
}
// Reinterpret the string as a slice, then cast to string which does a copy.
result := string(cSliceToUnsafeGoBytes(C.DBSlice{s.data, s.len}))
result := string(cSliceToUnsafeGoBytes(C.DBSlice(s)))
C.free(unsafe.Pointer(s.data))
return result
}
Expand Down
2 changes: 1 addition & 1 deletion pkg/cmd/roachprod/docker/Dockerfile
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
FROM golang:1.13
FROM golang:1.14
WORKDIR /build
COPY . .
RUN ["/build/build.sh"]
Expand Down
4 changes: 2 additions & 2 deletions pkg/geo/errors.go
Original file line number Diff line number Diff line change
Expand Up @@ -21,9 +21,9 @@ import (
func NewMismatchingSRIDsError(a GeospatialType, b GeospatialType) error {
return errors.Newf(
"operation on mixed SRIDs forbidden: (%s, %d) != (%s, %d)",
a.Shape(),
a.ShapeType(),
a.SRID(),
b.Shape(),
b.ShapeType(),
b.SRID(),
)
}
Expand Down
58 changes: 30 additions & 28 deletions pkg/geo/geo.go
Original file line number Diff line number Diff line change
Expand Up @@ -47,8 +47,8 @@ const (
type GeospatialType interface {
// SRID returns the SRID of the given type.
SRID() geopb.SRID
// Shape returns the Shape of the given type.
Shape() geopb.Shape
// ShapeType returns the ShapeType of the given type.
ShapeType() geopb.ShapeType
}

var _ GeospatialType = (*Geometry)(nil)
Expand All @@ -57,15 +57,17 @@ var _ GeospatialType = (*Geography)(nil)
// GeospatialTypeFitsColumnMetadata determines whether a GeospatialType is compatible with the
// given SRID and Shape.
// Returns an error if the types does not fit.
func GeospatialTypeFitsColumnMetadata(t GeospatialType, srid geopb.SRID, shape geopb.Shape) error {
func GeospatialTypeFitsColumnMetadata(
t GeospatialType, srid geopb.SRID, shapeType geopb.ShapeType,
) error {
// SRID 0 can take in any SRID. Otherwise SRIDs must match.
if srid != 0 && t.SRID() != srid {
return errors.Newf("object SRID %d does not match column SRID %d", t.SRID(), srid)
}
// Shape_Geometry/Shape_Unset can take in any kind of shape.
// Otherwise, shapes must match.
if shape != geopb.Shape_Unset && shape != geopb.Shape_Geometry && shape != t.Shape() {
return errors.Newf("object type %s does not match column type %s", t.Shape(), shape)
if shapeType != geopb.ShapeType_Unset && shapeType != geopb.ShapeType_Geometry && shapeType != t.ShapeType() {
return errors.Newf("object type %s does not match column type %s", t.ShapeType(), shapeType)
}
return nil
}
Expand Down Expand Up @@ -97,7 +99,7 @@ func NewGeometryUnsafe(spatialObject geopb.SpatialObject) *Geometry {

// NewGeometryFromPointCoords makes a point from x, y coordinates.
func NewGeometryFromPointCoords(x, y float64) (*Geometry, error) {
s, err := spatialObjectFromGeom(geom.NewPointFlat(geom.XY, []float64{x, y}))
s, err := spatialObjectFromGeomT(geom.NewPointFlat(geom.XY, []float64{x, y}))
if err != nil {
return nil, err
}
Expand All @@ -106,7 +108,7 @@ func NewGeometryFromPointCoords(x, y float64) (*Geometry, error) {

// NewGeometryFromGeom creates a new Geometry object from a geom.T object.
func NewGeometryFromGeom(g geom.T) (*Geometry, error) {
spatialObject, err := spatialObjectFromGeom(g)
spatialObject, err := spatialObjectFromGeomT(g)
if err != nil {
return nil, err
}
Expand Down Expand Up @@ -222,7 +224,7 @@ func adjustEWKBSRID(b geopb.EWKB, srid geopb.SRID) (geopb.SpatialObject, error)
return geopb.SpatialObject{}, err
}
adjustGeomSRID(t, srid)
return spatialObjectFromGeom(t)
return spatialObjectFromGeomT(t)
}

// AsGeomT returns the geometry as a geom.T object.
Expand Down Expand Up @@ -255,9 +257,9 @@ func (g *Geometry) SRID() geopb.SRID {
return g.spatialObject.SRID
}

// Shape returns the shape of the Geometry.
func (g *Geometry) Shape() geopb.Shape {
return g.spatialObject.Shape
// ShapeType returns the shape type of the Geometry.
func (g *Geometry) ShapeType() geopb.ShapeType {
return g.spatialObject.ShapeType
}

// BoundingBoxIntersects returns whether the bounding box of the given geometry
Expand Down Expand Up @@ -298,7 +300,7 @@ func NewGeographyUnsafe(spatialObject geopb.SpatialObject) *Geography {

// NewGeographyFromGeom creates a new Geography from a geom.T object.
func NewGeographyFromGeom(g geom.T) (*Geography, error) {
spatialObject, err := spatialObjectFromGeom(g)
spatialObject, err := spatialObjectFromGeomT(g)
if err != nil {
return nil, err
}
Expand Down Expand Up @@ -431,9 +433,9 @@ func (g *Geography) SRID() geopb.SRID {
return g.spatialObject.SRID
}

// Shape returns the shape of the Geography.
func (g *Geography) Shape() geopb.Shape {
return g.spatialObject.Shape
// ShapeType returns the shape type of the Geography.
func (g *Geography) ShapeType() geopb.ShapeType {
return g.spatialObject.ShapeType
}

// Spheroid returns the spheroid represented by the given Geography.
Expand Down Expand Up @@ -594,13 +596,13 @@ func S2RegionsFromGeom(geomRepr geom.T, emptyBehavior EmptyBehavior) ([]s2.Regio
// Common
//

// spatialObjectFromGeom creates a geopb.SpatialObject from a geom.T.
func spatialObjectFromGeom(t geom.T) (geopb.SpatialObject, error) {
// spatialObjectFromGeomT creates a geopb.SpatialObject from a geom.T.
func spatialObjectFromGeomT(t geom.T) (geopb.SpatialObject, error) {
ret, err := ewkb.Marshal(t, DefaultEWKBEncodingFormat)
if err != nil {
return geopb.SpatialObject{}, err
}
shape, err := shapeFromGeom(t)
shapeType, err := shapeTypeFromGeomT(t)
if err != nil {
return geopb.SpatialObject{}, err
}
Expand All @@ -617,29 +619,29 @@ func spatialObjectFromGeom(t geom.T) (geopb.SpatialObject, error) {
return geopb.SpatialObject{
EWKB: geopb.EWKB(ret),
SRID: geopb.SRID(t.SRID()),
Shape: shape,
ShapeType: shapeType,
BoundingBox: bbox,
}, nil
}

func shapeFromGeom(t geom.T) (geopb.Shape, error) {
func shapeTypeFromGeomT(t geom.T) (geopb.ShapeType, error) {
switch t := t.(type) {
case *geom.Point:
return geopb.Shape_Point, nil
return geopb.ShapeType_Point, nil
case *geom.LineString:
return geopb.Shape_LineString, nil
return geopb.ShapeType_LineString, nil
case *geom.Polygon:
return geopb.Shape_Polygon, nil
return geopb.ShapeType_Polygon, nil
case *geom.MultiPoint:
return geopb.Shape_MultiPoint, nil
return geopb.ShapeType_MultiPoint, nil
case *geom.MultiLineString:
return geopb.Shape_MultiLineString, nil
return geopb.ShapeType_MultiLineString, nil
case *geom.MultiPolygon:
return geopb.Shape_MultiPolygon, nil
return geopb.ShapeType_MultiPolygon, nil
case *geom.GeometryCollection:
return geopb.Shape_GeometryCollection, nil
return geopb.ShapeType_GeometryCollection, nil
default:
return geopb.Shape_Unset, errors.Newf("unknown shape: %T", t)
return geopb.ShapeType_Unset, errors.Newf("unknown shape: %T", t)
}
}

Expand Down
Loading

0 comments on commit 74d7021

Please sign in to comment.