Skip to content

Commit

Permalink
roachprod,roachtest: fix mounting on GCP and disable RAID0 in roachtests
Browse files Browse the repository at this point in the history 
Fixes bugs introduced by #72553, which ported support for multiple
stores from AWS to GCP.  This change fixes the bugs that caused
disks mounted to not be written to `/etc/fstab` on GCP instances,
causing mounts to go missing on instance restart.  Additionally, this
change modifies roachtest to default to creating multiple stores on GCP
in roachprod invocations rather than mounting a RAID 0 array, thus
preserving the existing behavior used in roachtests such as
`kv/multi-store-with-overload`.

Fixes #72635

Release note: None
  • Loading branch information
@AlexTalks
AlexTalks committed Nov 16, 2021
1 parent 774ea54 commit 8bb63ce
Show file tree
Hide file tree
Showing 6 changed files with 14 additions and 9 deletions.
5 changes: 5 additions & 0 deletions pkg/cmd/roachtest/spec/cluster_spec.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -173,6 +173,11 @@ func (s *ClusterSpec) Args(extra ...string) ([]string, error) {
switch s.Cloud {
case GCE:
args = append(args, fmt.Sprintf("--gce-local-ssd-count=%d", s.SSDs))

// NB: As the default behavior for roachprod (at least in AWS/GCP) is to mount multiple disks as a single
// store using a RAID 0 array, we must explicitly ask for multiple stores to be enabled. If a use case for
// using a RAID 0 array arises in roachtest, it can be added as an option in the ClusterSpec.
args = append(args, "--gce-enable-multiple-stores=true")
default:
return nil, errors.Errorf("specifying SSD count is not yet supported on %s", s.Cloud)
}
Expand Down
4 changes: 2 additions & 2 deletions pkg/cmd/roachtest/spec/testdata/collected_specs.txt
View file
Original file line number Diff line number Diff line change
Expand Up @@ -49,13 +49,13 @@ print-args-for-all
24: spec.ClusterSpec{Cloud:"gce", InstanceType:"", NodeCount:9, CPUs:1, SSDs:0, VolumeSize:0, PreferLocalSSD:true, Zones:"", Geo:false, Lifetime:43200000000000, ReusePolicy:spec.ReusePolicyAny{}, FileSystem:0, RandomlyUseZfs:false}
--clouds=gce --local-ssd=true --gce-machine-type=n1-standard-1 --lifetime=12h0m0s
25: spec.ClusterSpec{Cloud:"gce", InstanceType:"", NodeCount:6, CPUs:32, SSDs:2, VolumeSize:0, PreferLocalSSD:true, Zones:"", Geo:false, Lifetime:43200000000000, ReusePolicy:spec.ReusePolicyAny{}, FileSystem:0, RandomlyUseZfs:false}
--clouds=gce --local-ssd=true --gce-machine-type=n1-highcpu-32 --gce-local-ssd-count=2 --lifetime=12h0m0s
--clouds=gce --local-ssd=true --gce-machine-type=n1-highcpu-32 --gce-local-ssd-count=2 --gce-enable-multiple-stores=true --lifetime=12h0m0s
26: spec.ClusterSpec{Cloud:"gce", InstanceType:"", NodeCount:10, CPUs:16, SSDs:0, VolumeSize:0, PreferLocalSSD:true, Zones:"", Geo:false, Lifetime:43200000000000, ReusePolicy:spec.ReusePolicyAny{}, FileSystem:0, RandomlyUseZfs:false}
--clouds=gce --local-ssd=true --gce-machine-type=n1-highcpu-16 --lifetime=12h0m0s
27: spec.ClusterSpec{Cloud:"gce", InstanceType:"", NodeCount:4, CPUs:96, SSDs:0, VolumeSize:0, PreferLocalSSD:true, Zones:"", Geo:false, Lifetime:43200000000000, ReusePolicy:spec.ReusePolicyAny{}, FileSystem:0, RandomlyUseZfs:false}
--clouds=gce --local-ssd=true --gce-machine-type=n1-highcpu-96 --lifetime=12h0m0s
28: spec.ClusterSpec{Cloud:"gce", InstanceType:"", NodeCount:4, CPUs:4, SSDs:1, VolumeSize:0, PreferLocalSSD:true, Zones:"", Geo:false, Lifetime:43200000000000, ReusePolicy:spec.ReusePolicyAny{}, FileSystem:0, RandomlyUseZfs:false}
--clouds=gce --local-ssd=true --gce-machine-type=n1-standard-4 --gce-local-ssd-count=1 --lifetime=12h0m0s
--clouds=gce --local-ssd=true --gce-machine-type=n1-standard-4 --gce-local-ssd-count=1 --gce-enable-multiple-stores=true --lifetime=12h0m0s
29: spec.ClusterSpec{Cloud:"gce", InstanceType:"", NodeCount:2, CPUs:8, SSDs:0, VolumeSize:0, PreferLocalSSD:true, Zones:"", Geo:false, Lifetime:43200000000000, ReusePolicy:spec.ReusePolicyAny{}, FileSystem:0, RandomlyUseZfs:false}
--clouds=gce --local-ssd=true --gce-machine-type=n1-standard-8 --lifetime=12h0m0s
30: spec.ClusterSpec{Cloud:"gce", InstanceType:"", NodeCount:8, CPUs:4, SSDs:0, VolumeSize:0, PreferLocalSSD:true, Zones:"", Geo:false, Lifetime:43200000000000, ReusePolicy:spec.ReusePolicyAny{}, FileSystem:0, RandomlyUseZfs:false}
Expand Down
2 changes: 1 addition & 1 deletion pkg/roachprod/vm/aws/aws.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -301,7 +301,7 @@ func (o *ProviderOpts) ConfigureCreateFlags(flags *pflag.FlagSet) {
flags.StringVar(&o.ImageAMI, ProviderName+"-image-ami",
o.ImageAMI, "Override image AMI to use. See https://awscli.amazonaws.com/v2/documentation/api/latest/reference/ec2/describe-images.html")
flags.BoolVar(&o.UseMultipleDisks, ProviderName+"-enable-multiple-stores",
o.UseMultipleDisks, "Enable the use of multiple stores by creating one store directory per disk. "+
false, "Enable the use of multiple stores by creating one store directory per disk. "+
"Default is to raid0 stripe all disks. "+
"See repeating --"+ProviderName+"-ebs-volume for adding extra volumes.")
flags.Float64Var(&o.CreateRateLimit, ProviderName+"-create-rate-limit", o.CreateRateLimit, "aws"+
Expand Down
4 changes: 2 additions & 2 deletions pkg/roachprod/vm/aws/support.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,7 @@ mount_prefix="/mnt/data"
for d in $(ls /dev/nvme?n1 /dev/xvdd); do
if ! mount | grep ${d}; then
disks+=("${d}")
echo "Disk ${d} not mounted, creating..."
echo "Disk ${d} not mounted, need to mount..."
else
echo "Disk ${d} already mounted, skipping..."
fi
Expand All @@ -70,7 +70,7 @@ elif [ "${#disks[@]}" -eq "1" ] || [ -n "$use_multiple_disks" ]; then
do
mountpoint="${mount_prefix}${disknum}"
disknum=$((disknum + 1 ))
echo "Creating ${mountpoint}"
echo "Mounting ${disk} at ${mountpoint}"
mkdir -p ${mountpoint}
mkfs.ext4 -F ${disk}
mount -o ${mount_opts} ${disk} ${mountpoint}
Expand Down
2 changes: 1 addition & 1 deletion pkg/roachprod/vm/gce/gcloud.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -303,7 +303,7 @@ func (o *ProviderOpts) ConfigureCreateFlags(flags *pflag.FlagSet) {
flags.IntVar(&o.PDVolumeSize, ProviderName+"-pd-volume-size", 500,
"Size in GB of persistent disk volume, only used if local-ssd=false")
flags.BoolVar(&o.UseMultipleDisks, ProviderName+"-enable-multiple-stores",
o.UseMultipleDisks, "Enable the use of multiple stores by creating one store directory per disk. "+
false, "Enable the use of multiple stores by creating one store directory per disk. "+
"Default is to raid0 stripe all disks.")

flags.StringSliceVar(&o.Zones, ProviderName+"-zones", nil,
Expand Down
6 changes: 3 additions & 3 deletions pkg/roachprod/vm/gce/utils.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -73,7 +73,7 @@ for d in $(ls /dev/disk/by-id/google-local-* /dev/disk/by-id/google-persistent-d
if ! mount | grep ${d}; then
{{ end }}
disks+=("${d}")
echo "Disk ${d} not mounted, creating..."
echo "Disk ${d} not mounted, need to mount..."
else
echo "Disk ${d} already mounted, skipping..."
fi
Expand All @@ -90,15 +90,15 @@ elif [ "${#disks[@]}" -eq "1" ] || [ -n "$use_multiple_disks" ]; then
do
mountpoint="${mount_prefix}${disknum}"
disknum=$((disknum + 1 ))
echo "Creating ${mountpoint}"
echo "Mounting ${disk} at ${mountpoint}"
mkdir -p ${mountpoint}
{{ if .Zfs }}
zpool create -f $(basename $mountpoint) -m ${mountpoint} ${disk}
# NOTE: we don't need an /etc/fstab entry for ZFS. It will handle this itself.
{{ else }}
mkfs.ext4 -q -F ${disk}
mount -o ${mount_opts} ${disk} ${mountpoint}
echo "/dev/md0 ${mountpoint} ext4 ${mount_opts} 1 1" | tee -a /etc/fstab
echo "${d} ${mountpoint} ext4 ${mount_opts} 1 1" | tee -a /etc/fstab
{{ end }}
chmod 777 ${mountpoint}
done
Expand Down

0 comments on commit 8bb63ce

Please sign in to comment.