diff options
12 files changed, 427 insertions, 176 deletions
diff --git a/client/go/cmd/root.go b/client/go/cmd/root.go index 4202035af92..6bfca7fd613 100644 --- a/client/go/cmd/root.go +++ b/client/go/cmd/root.go @@ -49,6 +49,14 @@ const ( colorFlag = "color" ) +func isTerminal() bool { + file, ok := stdout.(*os.File) + if ok { + return isatty.IsTerminal(file.Fd()) + } + return false +} + func configureOutput() { log.SetFlags(0) // No timestamps log.SetOutput(stdout) @@ -65,10 +73,7 @@ func configureOutput() { colorize := false switch colorValue { case "auto": - file, ok := stdout.(*os.File) - if ok { - colorize = isatty.IsTerminal(file.Fd()) - } + colorize = isTerminal() case "always": colorize = true case "never": diff --git a/client/go/cmd/version.go b/client/go/cmd/version.go index 05820f4e34b..749d17a41d9 100644 --- a/client/go/cmd/version.go +++ b/client/go/cmd/version.go @@ -1,23 +1,144 @@ package cmd import ( + "encoding/json" "log" + "net/http" + "os" + "os/exec" + "path/filepath" "runtime" + "sort" + "strings" + "time" "github.com/spf13/cobra" "github.com/vespa-engine/vespa/client/go/build" + "github.com/vespa-engine/vespa/client/go/util" + "github.com/vespa-engine/vespa/client/go/version" ) +var skipVersionCheck bool + +var sp subprocess = &execSubprocess{} + +type subprocess interface { + pathOf(name string) (string, error) + outputOf(name string, args ...string) ([]byte, error) + isTerminal() bool +} + +type execSubprocess struct{} + +func (c *execSubprocess) pathOf(name string) (string, error) { return exec.LookPath(name) } +func (c *execSubprocess) isTerminal() bool { return isTerminal() } +func (c *execSubprocess) outputOf(name string, args ...string) ([]byte, error) { + return exec.Command(name, args...).Output() +} + func init() { rootCmd.AddCommand(versionCmd) + versionCmd.Flags().BoolVarP(&skipVersionCheck, "no-check", "n", false, "Do not check if a new version is available") } var versionCmd = &cobra.Command{ Use: "version", - Short: "Show version number", + Short: "Show current version and check for updates", DisableAutoGenTag: true, Args: cobra.ExactArgs(0), Run: func(cmd *cobra.Command, args []string) { log.Printf("vespa version %s compiled with %v on %v/%v", build.Version, runtime.Version(), runtime.GOOS, runtime.GOARCH) + if !skipVersionCheck && sp.isTerminal() { + if err := checkVersion(); err != nil { + fatalErr(err) + } + } }, } + +func checkVersion() error { + current, err := version.Parse(build.Version) + if err != nil { + return err + } + latest, err := latestRelease() + if err != nil { + return err + } + if !current.Less(latest.Version) { + return nil + } + usingHomebrew := usingHomebrew() + if usingHomebrew && latest.isRecent() { + return nil // Allow some time for new release to appear in Homebrew repo + } + log.Printf("\nNew release available: %s", color.Green(latest.Version)) + log.Printf("https://github.com/vespa-engine/vespa/releases/tag/v%s", latest.Version) + if usingHomebrew { + log.Printf("\nUpgrade by running:\n%s", color.Cyan("brew update && brew upgrade vespa-cli")) + } + return nil +} + +func latestRelease() (release, error) { + req, err := http.NewRequest("GET", "https://api.github.com/repos/vespa-engine/vespa/releases", nil) + if err != nil { + return release{}, err + } + response, err := util.HttpDo(req, time.Minute, "GitHub") + if err != nil { + return release{}, err + } + defer response.Body.Close() + + var ghReleases []githubRelease + dec := json.NewDecoder(response.Body) + if err := dec.Decode(&ghReleases); err != nil { + return release{}, err + } + if len(ghReleases) == 0 { + return release{}, nil // No releases found + } + + var releases []release + for _, r := range ghReleases { + v, err := version.Parse(r.TagName) + if err != nil { + return release{}, err + } + publishedAt, err := time.Parse(time.RFC3339, r.PublishedAt) + if err != nil { + return release{}, err + } + releases = append(releases, release{Version: v, PublishedAt: publishedAt}) + } + sort.Slice(releases, func(i, j int) bool { return releases[i].Version.Less(releases[j].Version) }) + return releases[len(releases)-1], nil +} + +func usingHomebrew() bool { + selfPath, err := sp.pathOf("vespa") + if err != nil { + return false + } + brewPrefix, err := sp.outputOf("brew", "--prefix") + if err != nil { + return false + } + brewBin := filepath.Join(strings.TrimSpace(string(brewPrefix)), "bin") + string(os.PathSeparator) + return strings.HasPrefix(selfPath, brewBin) +} + +type githubRelease struct { + TagName string `json:"tag_name"` + PublishedAt string `json:"published_at"` +} + +type release struct { + Version version.Version + PublishedAt time.Time +} + +func (r release) isRecent() bool { + return time.Now().Before(r.PublishedAt.Add(time.Hour * 24)) +} diff --git a/client/go/cmd/version_test.go b/client/go/cmd/version_test.go index fc977c47938..3b0f73de408 100644 --- a/client/go/cmd/version_test.go +++ b/client/go/cmd/version_test.go @@ -1,11 +1,51 @@ package cmd import ( + "fmt" "testing" "github.com/stretchr/testify/assert" + "github.com/vespa-engine/vespa/client/go/util" ) func TestVersion(t *testing.T) { - assert.Contains(t, execute(command{args: []string{"version"}}, t, nil), "vespa version 0.0.0-devel compiled with") + c := &mockHttpClient{} + c.NextResponse(200, `[{"tag_name": "v1.2.3", "published_at": "2021-09-10T12:00:00Z"}]`) + util.ActiveHttpClient = c + + sp = &mockSubprocess{} + out := execute(command{args: []string{"version"}}, t, nil) + assert.Contains(t, out, "vespa version 0.0.0-devel compiled with") + assert.Contains(t, out, "New release available: 1.2.3\nhttps://github.com/vespa-engine/vespa/releases/tag/v1.2.3") +} + +func TestVersionCheckHomebrew(t *testing.T) { + c := &mockHttpClient{} + c.NextResponse(200, `[{"tag_name": "v1.2.3", "published_at": "2021-09-10T12:00:00Z"}]`) + util.ActiveHttpClient = c + + sp = &mockSubprocess{programPath: "/usr/local/bin/vespa", output: "/usr/local"} + out := execute(command{args: []string{"version"}}, t, nil) + assert.Contains(t, out, "vespa version 0.0.0-devel compiled with") + assert.Contains(t, out, "New release available: 1.2.3\n"+ + "https://github.com/vespa-engine/vespa/releases/tag/v1.2.3\n"+ + "\nUpgrade by running:\nbrew update && brew upgrade vespa-cli\n") +} + +type mockSubprocess struct { + programPath string + output string } + +func (c *mockSubprocess) pathOf(name string) (string, error) { + if c.programPath == "" { + return "", fmt.Errorf("no program path set in this mock") + } + return c.programPath, nil +} + +func (c *mockSubprocess) outputOf(name string, args ...string) ([]byte, error) { + return []byte(c.output), nil +} + +func (c *mockSubprocess) isTerminal() bool { return true } diff --git a/client/go/version/version.go b/client/go/version/version.go new file mode 100644 index 00000000000..27b7da1d0f5 --- /dev/null +++ b/client/go/version/version.go @@ -0,0 +1,92 @@ +package version + +import ( + "fmt" + "strconv" + "strings" +) + +// Version represents a semantic version number. +type Version struct { + Major int + Minor int + Patch int + Label string +} + +func (v Version) String() string { + var sb strings.Builder + sb.WriteString(strconv.Itoa(v.Major)) + sb.WriteRune('.') + sb.WriteString(strconv.Itoa(v.Minor)) + sb.WriteRune('.') + sb.WriteString(strconv.Itoa(v.Patch)) + if v.Label != "" { + sb.WriteRune('-') + sb.WriteString(v.Label) + } + return sb.String() +} + +// Compare returns a positive integer if v1 is greater than v2, a negative integer if v1 is less than v2 and zero if they +// are equal. +func (v1 Version) Compare(v2 Version) int { + result := v1.Major - v2.Major + if result != 0 { + return result + } + result = v1.Minor - v2.Minor + if result != 0 { + return result + } + result = v1.Patch - v2.Patch + if result != 0 { + return result + } + // Version without label always sorts first + if v1.Label == "" && v2.Label != "" { + return 1 + } + if v1.Label != "" && v2.Label == "" { + return -1 + } + if v1.Label > v2.Label { + return 1 + } + if v1.Label < v2.Label { + return -1 + } + return 0 +} + +// Less returns true if v1 is lower than v2. +func (v1 Version) Less(v2 Version) bool { return v1.Compare(v2) < 0 } + +// Parse parses a semantic version number from string s. +func Parse(s string) (Version, error) { + if len(s) > 0 && s[0] == 'v' { + s = s[1:] // Trim v prefix + } + parts := strings.Split(s, ".") + if len(parts) != 3 { + return Version{}, fmt.Errorf("invalid version number: %s", s) + } + major, err := strconv.Atoi(parts[0]) + if err != nil { + return Version{}, fmt.Errorf("invalid major version: %s", parts[0]) + } + minor, err := strconv.Atoi(parts[1]) + if err != nil { + return Version{}, fmt.Errorf("invalid minor version: %s", parts[1]) + } + parts2 := strings.SplitN(parts[2], "-", 2) + patch, err := strconv.Atoi(parts2[0]) + if err != nil { + return Version{}, fmt.Errorf("invalid patch version: %s", parts[2]) + } + v := Version{Major: major, Minor: minor, Patch: patch} + if len(parts2) > 1 { + v.Label = parts2[1] + } + return v, nil +} diff --git a/client/go/version/version_test.go b/client/go/version/version_test.go new file mode 100644 index 00000000000..3602715cca8 --- /dev/null +++ b/client/go/version/version_test.go @@ -0,0 +1,66 @@ +package version + +import ( + "fmt" + "testing" + + "github.com/stretchr/testify/assert" +) + +func TestParse(t *testing.T) { + _, err := Parse("foo") + assert.NotNil(t, err) + + v, err := Parse("1.2.3") + assert.Nil(t, err) + assert.Equal(t, "1.2.3", v.String()) + + v, err = Parse("v4.5.6") + assert.Nil(t, err) + assert.Equal(t, "4.5.6", v.String()) + + v, err = Parse("1.2.3-foo") + assert.Nil(t, err) + assert.Equal(t, "1.2.3-foo", v.String()) +} + +func TestCompare(t *testing.T) { + assertComparison(t, "1.2.3", '>', "1.0.0") + assertComparison(t, "1.0.0", '<', "1.2.3") + + assertComparison(t, "1.2.3", '=', "1.2.3") + assertComparison(t, "1.2.3", '>', "1.2.0") + assertComparison(t, "1.2.0", '<', "1.2.3") + assertComparison(t, "1.2.3", '>', "1.2.0") + assertComparison(t, "1.0.3", '<', "1.2.3") + + assertComparison(t, "1.2.3", '>', "1.1.4") + assertComparison(t, "1.1.4", '<', "1.2.3") + + assertComparison(t, "2.0.0", '>', "1.2.3") + assertComparison(t, "1.2.3", '<', "2.0.0") + + assertComparison(t, "1.2.3-alpha1", '<', "1.2.3") + assertComparison(t, "1.2.3", '>', "1.2.3-alpha1") + assertComparison(t, "1.2.3-alpha1", '=', "1.2.3-alpha1") + assertComparison(t, "1.2.3-alpha1", '<', "1.2.3-alpha2") + assertComparison(t, "1.2.3-alpha2", '>', "1.2.3-alpha1") +} + +func assertComparison(t *testing.T, s1 string, cmp rune, s2 string) { + v1, err := Parse(s1) + assert.Nil(t, err) + v2, err := Parse(s2) + assert.Nil(t, err) + result := v1.Compare(v2) + switch cmp { + case '<': + assert.True(t, result < 0, fmt.Sprintf("%s is less than %s", v1, v2)) + case '>': + assert.True(t, result > 0, fmt.Sprintf("%s is greater than %s", v1, v2)) + case '=': + assert.True(t, result == 0, fmt.Sprintf("%s is equal to %s", v1, v2)) + default: + t.Fatal("invalid comparator: %r", cmp) + } +} diff --git a/config-model/src/main/java/com/yahoo/searchdefinition/derived/RankProfileList.java b/config-model/src/main/java/com/yahoo/searchdefinition/derived/RankProfileList.java index 461b9109dfa..46b785ccf42 100644 --- a/config-model/src/main/java/com/yahoo/searchdefinition/derived/RankProfileList.java +++ b/config-model/src/main/java/com/yahoo/searchdefinition/derived/RankProfileList.java @@ -20,7 +20,6 @@ import com.yahoo.vespa.model.AbstractService; import java.util.ArrayList; import java.util.Collection; -import java.util.HashMap; import java.util.LinkedHashMap; import java.util.List; import java.util.Map; @@ -101,6 +100,12 @@ public class RankProfileList extends Derived implements RankProfilesConfig.Produ remaining.forEach((name, rank) -> { if (areDependenciesReady(rank, rankProfileRegistry)) ready.add(rank); }); + if (ready.isEmpty() && ! deployProperties.featureFlags().enforceRankProfileInheritance()) { + // Dirty fallback to allow incorrect rankprofile inheritance to pass for now. + // We then handle one by one. + // TODO remove ASAP + ready.add(remaining.values().iterator().next()); + } processRankProfiles(ready, queryProfiles, importedModels, search, attributeFields, deployProperties, executor); ready.forEach(rank -> remaining.remove(rank.getName())); } diff --git a/flags/src/main/java/com/yahoo/vespa/flags/Flags.java b/flags/src/main/java/com/yahoo/vespa/flags/Flags.java index 7c9ee425d8f..6a3b33e6458 100644 --- a/flags/src/main/java/com/yahoo/vespa/flags/Flags.java +++ b/flags/src/main/java/com/yahoo/vespa/flags/Flags.java @@ -134,13 +134,6 @@ public class Flags { "Number of threads used for speeding up building of models.", "Takes effect on first (re)start of config server"); - public static final UnboundBooleanFlag GROUP_PERMANENT_SUSPENSION = defineFeatureFlag( - "group-permanent-suspension", true, - List.of("hakonhall"), "2021-09-11", "2021-11-11", - "Allow all content nodes in a hierarchical group to suspend at the same time when" + - "permanently suspending a host.", - "Takes effect on the next permanent suspension request to the Orchestrator."); - public static final UnboundBooleanFlag ENCRYPT_DIRTY_DISK = defineFeatureFlag( "encrypt-dirty-disk", false, List.of("hakonhall"), "2021-05-14", "2021-10-05", @@ -160,7 +153,7 @@ public class Flags { "Takes effect on first run of DiskTask, typically after host-admin restart/upgrade."); public static final UnboundBooleanFlag USE_UNKNOWN_SERVICE_STATUS = defineFeatureFlag( - "use-unknown-service-status", false, + "use-unknown-service-status", true, List.of("hakonhall"), "2021-09-13", "2021-10-13", "Whether to use the UNKNOWN ServiceStatus for services that have not yet been probed by service monitor.", "Takes effect on first (re)start of config server."); diff --git a/orchestrator/src/main/java/com/yahoo/vespa/orchestrator/policy/HostedVespaClusterPolicy.java b/orchestrator/src/main/java/com/yahoo/vespa/orchestrator/policy/HostedVespaClusterPolicy.java index d183e863500..208e12690ff 100644 --- a/orchestrator/src/main/java/com/yahoo/vespa/orchestrator/policy/HostedVespaClusterPolicy.java +++ b/orchestrator/src/main/java/com/yahoo/vespa/orchestrator/policy/HostedVespaClusterPolicy.java @@ -4,9 +4,7 @@ package com.yahoo.vespa.orchestrator.policy; import com.yahoo.config.provision.Zone; import com.yahoo.vespa.applicationmodel.ClusterId; import com.yahoo.vespa.applicationmodel.ServiceType; -import com.yahoo.vespa.flags.BooleanFlag; import com.yahoo.vespa.flags.FlagSource; -import com.yahoo.vespa.flags.Flags; import com.yahoo.vespa.orchestrator.model.ClusterApi; import com.yahoo.vespa.orchestrator.model.VespaModelUtil; @@ -17,12 +15,9 @@ import static com.yahoo.vespa.orchestrator.policy.HostedVespaPolicy.ENOUGH_SERVI public class HostedVespaClusterPolicy implements ClusterPolicy { - private final BooleanFlag groupSuspensionInPermanentSuspendFlag; private final Zone zone; public HostedVespaClusterPolicy(FlagSource flagSource, Zone zone) { - // Note that the "group" in this flag refers to hierarchical groups of a content cluster. - this.groupSuspensionInPermanentSuspendFlag = Flags.GROUP_PERMANENT_SUSPENSION.bindTo(flagSource); this.zone = zone; } @@ -32,7 +27,7 @@ public class HostedVespaClusterPolicy implements ClusterPolicy { return SuspensionReasons.nothingNoteworthy(); } - int percentageOfServicesAllowedToBeDown = getConcurrentSuspensionLimit(clusterApi, true).asPercentage(); + int percentageOfServicesAllowedToBeDown = getConcurrentSuspensionLimit(clusterApi).asPercentage(); if (clusterApi.percentageOfServicesDownIfGroupIsAllowedToBeDown() <= percentageOfServicesAllowedToBeDown) { return SuspensionReasons.nothingNoteworthy(); } @@ -63,9 +58,7 @@ public class HostedVespaClusterPolicy implements ClusterPolicy { return; } - boolean enableContentGroupSuspension = groupSuspensionInPermanentSuspendFlag.value(); - - int percentageOfServicesAllowedToBeDown = getConcurrentSuspensionLimit(clusterApi, enableContentGroupSuspension) + int percentageOfServicesAllowedToBeDown = getConcurrentSuspensionLimit(clusterApi) .asPercentage(); if (clusterApi.percentageOfServicesDownIfGroupIsAllowedToBeDown() <= percentageOfServicesAllowedToBeDown) { return; @@ -81,116 +74,85 @@ public class HostedVespaClusterPolicy implements ClusterPolicy { } // Non-private for testing purposes - ConcurrentSuspensionLimitForCluster getConcurrentSuspensionLimit(ClusterApi clusterApi, boolean enableContentGroupSuspension) { - if (enableContentGroupSuspension) { - // Possible service clusters on a node as of 2021-01-22: - // - // CLUSTER ID SERVICE TYPE HEALTH ASSOCIATION - // 1 CCN-controllers container-clustercontrollers Slobrok 1, 3, or 6 in content cluster - // 2 CCN distributor Slobrok content cluster - // 3 CCN storagenode Slobrok content cluster - // 4 CCN searchnode Slobrok content cluster - // 5 CCN transactionlogserver not checked content cluster - // 6 JCCN container Slobrok jdisc container cluster - // 7 admin slobrok not checked 1-3 in jdisc container cluster - // 8 metrics metricsproxy-container Slobrok application - // 9 admin logd not checked application - // 10 admin config-sentinel not checked application - // 11 admin configproxy not checked application - // 12 admin logforwarder not checked application - // 13 controller controller state/v1 controllers - // 14 zone-config-servers configserver state/v1 config servers - // 15 controller-host hostadmin state/v1 controller hosts - // 16 configserver-host hostadmin state/v1 config server hosts - // 17 tenant-host hostadmin state/v1 tenant hosts - // 18 proxy-host hostadmin state/v1 proxy hosts - // - // CCN refers to the content cluster's name, as specified in services.xml. - // JCCN refers to the jdisc container cluster's name, as specified in services.xml. - // - // For instance a content node will have 2-5 and 8-12 and possibly 1, while a combined - // cluster node may have all 1-12. - // - // The services on a node can be categorized into these main types, ref association column above: - // A content - // B container - // C tenant host - // D config server - // E config server host - // F controller - // G controller host - // H proxy (same as B) - // I proxy host - - if (clusterApi.serviceType().equals(ServiceType.CLUSTER_CONTROLLER)) { - return ConcurrentSuspensionLimitForCluster.ONE_NODE; - } - - if (Set.of(ServiceType.STORAGE, ServiceType.SEARCH, ServiceType.DISTRIBUTOR, ServiceType.TRANSACTION_LOG_SERVER) - .contains(clusterApi.serviceType())) { - // Delegate to the cluster controller - return ConcurrentSuspensionLimitForCluster.ALL_NODES; - } - - if (clusterApi.serviceType().equals(ServiceType.CONTAINER)) { - return ConcurrentSuspensionLimitForCluster.TEN_PERCENT; - } - - if (VespaModelUtil.ADMIN_CLUSTER_ID.equals(clusterApi.clusterId())) { - if (ServiceType.SLOBROK.equals(clusterApi.serviceType())) { - return ConcurrentSuspensionLimitForCluster.ONE_NODE; - } - - return ConcurrentSuspensionLimitForCluster.ALL_NODES; - } else if (ServiceType.METRICS_PROXY.equals(clusterApi.serviceType())) { - return ConcurrentSuspensionLimitForCluster.ALL_NODES; - } - - if (Set.of(ServiceType.CONFIG_SERVER, ServiceType.CONTROLLER).contains(clusterApi.serviceType())) { - return ConcurrentSuspensionLimitForCluster.ONE_NODE; - } - - if (clusterApi.serviceType().equals(ServiceType.HOST_ADMIN)) { - if (Set.of(ClusterId.CONFIG_SERVER_HOST, ClusterId.CONTROLLER_HOST).contains(clusterApi.clusterId())) { - return ConcurrentSuspensionLimitForCluster.ONE_NODE; - } + ConcurrentSuspensionLimitForCluster getConcurrentSuspensionLimit(ClusterApi clusterApi) { + // Possible service clusters on a node as of 2021-01-22: + // + // CLUSTER ID SERVICE TYPE HEALTH ASSOCIATION + // 1 CCN-controllers container-clustercontrollers Slobrok 1, 3, or 6 in content cluster + // 2 CCN distributor Slobrok content cluster + // 3 CCN storagenode Slobrok content cluster + // 4 CCN searchnode Slobrok content cluster + // 5 CCN transactionlogserver not checked content cluster + // 6 JCCN container Slobrok jdisc container cluster + // 7 admin slobrok not checked 1-3 in jdisc container cluster + // 8 metrics metricsproxy-container Slobrok application + // 9 admin logd not checked application + // 10 admin config-sentinel not checked application + // 11 admin configproxy not checked application + // 12 admin logforwarder not checked application + // 13 controller controller state/v1 controllers + // 14 zone-config-servers configserver state/v1 config servers + // 15 controller-host hostadmin state/v1 controller hosts + // 16 configserver-host hostadmin state/v1 config server hosts + // 17 tenant-host hostadmin state/v1 tenant hosts + // 18 proxy-host hostadmin state/v1 proxy hosts + // + // CCN refers to the content cluster's name, as specified in services.xml. + // JCCN refers to the jdisc container cluster's name, as specified in services.xml. + // + // For instance a content node will have 2-5 and 8-12 and possibly 1, while a combined + // cluster node may have all 1-12. + // + // The services on a node can be categorized into these main types, ref association column above: + // A content + // B container + // C tenant host + // D config server + // E config server host + // F controller + // G controller host + // H proxy (same as B) + // I proxy host + + if (clusterApi.serviceType().equals(ServiceType.CLUSTER_CONTROLLER)) { + return ConcurrentSuspensionLimitForCluster.ONE_NODE; + } - return zone.system().isCd() - ? ConcurrentSuspensionLimitForCluster.FIFTY_PERCENT - : ConcurrentSuspensionLimitForCluster.TWENTY_PERCENT; - } + if (Set.of(ServiceType.STORAGE, ServiceType.SEARCH, ServiceType.DISTRIBUTOR, ServiceType.TRANSACTION_LOG_SERVER) + .contains(clusterApi.serviceType())) { + // Delegate to the cluster controller + return ConcurrentSuspensionLimitForCluster.ALL_NODES; + } - // The above should cover all cases, but if not we'll return a reasonable default: + if (clusterApi.serviceType().equals(ServiceType.CONTAINER)) { return ConcurrentSuspensionLimitForCluster.TEN_PERCENT; - } else { - // TODO: Remove this legacy branch - if (clusterApi.isStorageCluster()) { - return ConcurrentSuspensionLimitForCluster.ONE_NODE; - } + } - if (ServiceType.CLUSTER_CONTROLLER.equals(clusterApi.serviceType())) { + if (VespaModelUtil.ADMIN_CLUSTER_ID.equals(clusterApi.clusterId())) { + if (ServiceType.SLOBROK.equals(clusterApi.serviceType())) { return ConcurrentSuspensionLimitForCluster.ONE_NODE; } - if (ServiceType.METRICS_PROXY.equals(clusterApi.serviceType())) { - return ConcurrentSuspensionLimitForCluster.ALL_NODES; - } - - if (VespaModelUtil.ADMIN_CLUSTER_ID.equals(clusterApi.clusterId())) { - if (ServiceType.SLOBROK.equals(clusterApi.serviceType())) { - return ConcurrentSuspensionLimitForCluster.ONE_NODE; - } + return ConcurrentSuspensionLimitForCluster.ALL_NODES; + } else if (ServiceType.METRICS_PROXY.equals(clusterApi.serviceType())) { + return ConcurrentSuspensionLimitForCluster.ALL_NODES; + } - return ConcurrentSuspensionLimitForCluster.ALL_NODES; - } + if (Set.of(ServiceType.CONFIG_SERVER, ServiceType.CONTROLLER).contains(clusterApi.serviceType())) { + return ConcurrentSuspensionLimitForCluster.ONE_NODE; + } - if (clusterApi.getApplication().applicationId().equals(VespaModelUtil.TENANT_HOST_APPLICATION_ID)) { - return zone.system().isCd() - ? ConcurrentSuspensionLimitForCluster.FIFTY_PERCENT - : ConcurrentSuspensionLimitForCluster.TWENTY_PERCENT; + if (clusterApi.serviceType().equals(ServiceType.HOST_ADMIN)) { + if (Set.of(ClusterId.CONFIG_SERVER_HOST, ClusterId.CONTROLLER_HOST).contains(clusterApi.clusterId())) { + return ConcurrentSuspensionLimitForCluster.ONE_NODE; } - return ConcurrentSuspensionLimitForCluster.TEN_PERCENT; + return zone.system().isCd() + ? ConcurrentSuspensionLimitForCluster.FIFTY_PERCENT + : ConcurrentSuspensionLimitForCluster.TWENTY_PERCENT; } + + // The above should cover all cases, but if not we'll return a reasonable default: + return ConcurrentSuspensionLimitForCluster.TEN_PERCENT; } } diff --git a/orchestrator/src/test/java/com/yahoo/vespa/orchestrator/model/ClusterApiImplTest.java b/orchestrator/src/test/java/com/yahoo/vespa/orchestrator/model/ClusterApiImplTest.java index 1e29f0ca5de..da8591c6631 100644 --- a/orchestrator/src/test/java/com/yahoo/vespa/orchestrator/model/ClusterApiImplTest.java +++ b/orchestrator/src/test/java/com/yahoo/vespa/orchestrator/model/ClusterApiImplTest.java @@ -10,7 +10,6 @@ import com.yahoo.vespa.applicationmodel.ServiceInstance; import com.yahoo.vespa.applicationmodel.ServiceStatus; import com.yahoo.vespa.applicationmodel.ServiceStatusInfo; import com.yahoo.vespa.applicationmodel.ServiceType; -import com.yahoo.vespa.flags.Flags; import com.yahoo.vespa.flags.InMemoryFlagSource; import com.yahoo.vespa.orchestrator.OrchestratorUtil; import com.yahoo.vespa.orchestrator.policy.ClusterParams; @@ -182,18 +181,6 @@ public class ClusterApiImplTest { fail(); } catch (HostStateChangeDeniedException e) { assertThat(e.getMessage(), - containsString("Changing the state of cfg1 would violate enough-services-up: " + - "Suspension of service with type 'configserver' not allowed: 33% are suspended already. " + - "Services down on resumed hosts: [1 missing config server].")); - } - - flagSource.withBooleanFlag(Flags.GROUP_PERMANENT_SUSPENSION.id(), true); - - try { - policy.verifyGroupGoingDownIsFine(clusterApi); - fail(); - } catch (HostStateChangeDeniedException e) { - assertThat(e.getMessage(), containsString("Suspension of service with type 'configserver' not allowed: 33% are suspended already. " + "Services down on resumed hosts: [1 missing config server].")); } @@ -214,18 +201,6 @@ public class ClusterApiImplTest { fail(); } catch (HostStateChangeDeniedException e) { assertThat(e.getMessage(), - containsString("Changing the state of cfg1 would violate enough-services-up: " + - "Suspension of service with type 'hostadmin' not allowed: 33% are suspended already. " + - "Services down on resumed hosts: [1 missing config server host].")); - } - - flagSource.withBooleanFlag(Flags.GROUP_PERMANENT_SUSPENSION.id(), true); - - try { - policy.verifyGroupGoingDownIsFine(clusterApi); - fail(); - } catch (HostStateChangeDeniedException e) { - assertThat(e.getMessage(), containsString("Suspension of service with type 'hostadmin' not allowed: 33% are suspended already. " + "Services down on resumed hosts: [1 missing config server host].")); } diff --git a/orchestrator/src/test/java/com/yahoo/vespa/orchestrator/policy/HostedVespaClusterPolicyTest.java b/orchestrator/src/test/java/com/yahoo/vespa/orchestrator/policy/HostedVespaClusterPolicyTest.java index 0c3da1656bc..303dabebba8 100644 --- a/orchestrator/src/test/java/com/yahoo/vespa/orchestrator/policy/HostedVespaClusterPolicyTest.java +++ b/orchestrator/src/test/java/com/yahoo/vespa/orchestrator/policy/HostedVespaClusterPolicyTest.java @@ -63,7 +63,7 @@ public class HostedVespaClusterPolicyTest { when(clusterApi.clusterId()).thenReturn(VespaModelUtil.ADMIN_CLUSTER_ID); when(clusterApi.serviceType()).thenReturn(ServiceType.SLOBROK); assertEquals(ConcurrentSuspensionLimitForCluster.ONE_NODE, - policy.getConcurrentSuspensionLimit(clusterApi, false)); + policy.getConcurrentSuspensionLimit(clusterApi)); } @Test @@ -71,46 +71,38 @@ public class HostedVespaClusterPolicyTest { when(clusterApi.clusterId()).thenReturn(VespaModelUtil.ADMIN_CLUSTER_ID); when(clusterApi.serviceType()).thenReturn(new ServiceType("non-slobrok-service-type")); assertEquals(ConcurrentSuspensionLimitForCluster.ALL_NODES, - policy.getConcurrentSuspensionLimit(clusterApi, false)); + policy.getConcurrentSuspensionLimit(clusterApi)); } @Test public void testStorageSuspensionLimit() { when(clusterApi.serviceType()).thenReturn(ServiceType.STORAGE); when(clusterApi.clusterId()).thenReturn(new ClusterId("some-cluster-id")); - when(clusterApi.isStorageCluster()).thenReturn(true); assertEquals(ConcurrentSuspensionLimitForCluster.ALL_NODES, - policy.getConcurrentSuspensionLimit(clusterApi, true)); - } - - @Test - public void testStorageSuspensionLimit_legacy() { - when(clusterApi.clusterId()).thenReturn(new ClusterId("some-cluster-id")); - when(clusterApi.isStorageCluster()).thenReturn(true); - assertEquals(ConcurrentSuspensionLimitForCluster.ONE_NODE, - policy.getConcurrentSuspensionLimit(clusterApi, false)); + policy.getConcurrentSuspensionLimit(clusterApi)); } @Test public void testTenantHostSuspensionLimit() { when(applicationApi.applicationId()).thenReturn(VespaModelUtil.TENANT_HOST_APPLICATION_ID); - when(clusterApi.isStorageCluster()).thenReturn(false); + when(clusterApi.clusterId()).thenReturn(ClusterId.TENANT_HOST); + when(clusterApi.serviceType()).thenReturn(ServiceType.HOST_ADMIN); assertEquals(ConcurrentSuspensionLimitForCluster.TWENTY_PERCENT, - policy.getConcurrentSuspensionLimit(clusterApi, false)); + policy.getConcurrentSuspensionLimit(clusterApi)); when(zone.system()).thenReturn(SystemName.cd); assertEquals(ConcurrentSuspensionLimitForCluster.FIFTY_PERCENT, - policy.getConcurrentSuspensionLimit(clusterApi, false)); + policy.getConcurrentSuspensionLimit(clusterApi)); } @Test public void testDefaultSuspensionLimit() { when(applicationApi.applicationId()).thenReturn(ApplicationId.fromSerializedForm("a:b:c")); when(clusterApi.clusterId()).thenReturn(new ClusterId("some-cluster-id")); - when(clusterApi.isStorageCluster()).thenReturn(false); + when(clusterApi.serviceType()).thenReturn(new ServiceType("some-service-type")); assertEquals(ConcurrentSuspensionLimitForCluster.TEN_PERCENT, - policy.getConcurrentSuspensionLimit(clusterApi, false)); + policy.getConcurrentSuspensionLimit(clusterApi)); } @Test @@ -141,7 +133,7 @@ public class HostedVespaClusterPolicyTest { when(clusterApi.noServicesOutsideGroupIsDown()).thenReturn(noServicesOutsideGroupIsDown); when(clusterApi.reasonsForNoServicesInGroupIsUp()).thenReturn(noServicesInGroupIsUp); when(clusterApi.percentageOfServicesDownIfGroupIsAllowedToBeDown()).thenReturn(20); - doReturn(ConcurrentSuspensionLimitForCluster.TEN_PERCENT).when(policy).getConcurrentSuspensionLimit(clusterApi, false); + doReturn(ConcurrentSuspensionLimitForCluster.TEN_PERCENT).when(policy).getConcurrentSuspensionLimit(clusterApi); when(applicationApi.applicationId()).thenReturn(ApplicationId.fromSerializedForm("a:b:c")); when(clusterApi.serviceType()).thenReturn(new ServiceType("service-type")); diff --git a/streamingvisitors/src/vespa/searchvisitor/searchenvironment.cpp b/streamingvisitors/src/vespa/searchvisitor/searchenvironment.cpp index 614d11aabfa..623de8d85d1 100644 --- a/streamingvisitors/src/vespa/searchvisitor/searchenvironment.cpp +++ b/streamingvisitors/src/vespa/searchvisitor/searchenvironment.cpp @@ -16,8 +16,8 @@ __thread SearchEnvironment::EnvMap * SearchEnvironment::_localEnvMap=0; SearchEnvironment::Env::Env(const vespalib::string & muffens, const config::ConfigUri & configUri, Fast_NormalizeWordFolder & wf) : _configId(configUri.getConfigId()), _configurer(std::make_unique<config::SimpleConfigRetriever>(createKeySet(configUri.getConfigId()), configUri.getContext()), this), - _vsmAdapter(new VSMAdapter(muffens, _configId, wf)), - _rankManager(new RankManager(_vsmAdapter.get())) + _vsmAdapter(std::make_unique<VSMAdapter>(muffens, _configId, wf)), + _rankManager(std::make_unique<RankManager>(_vsmAdapter.get())) { _configurer.start(); diff --git a/streamingvisitors/src/vespa/searchvisitor/searchvisitor.cpp b/streamingvisitors/src/vespa/searchvisitor/searchvisitor.cpp index 5e1e95b4681..a1ec153d7da 100644 --- a/streamingvisitors/src/vespa/searchvisitor/searchvisitor.cpp +++ b/streamingvisitors/src/vespa/searchvisitor/searchvisitor.cpp @@ -180,7 +180,7 @@ SearchVisitor::SearchVisitor(StorageComponent& component, _hitCount(0), _hitsRejectedCount(0), _query(), - _queryResult(new documentapi::QueryResultMessage()), + _queryResult(std::make_unique<documentapi::QueryResultMessage>()), _fieldSearcherMap(), _docTypeMapping(), _fieldSearchSpecMap(), @@ -192,10 +192,10 @@ SearchVisitor::SearchVisitor(StorageComponent& component, _groupingList(), _attributeFields(), _sortList(), - _searchBuffer(new vsm::SearcherBuf()), + _searchBuffer(std::make_shared<vsm::SearcherBuf>()), _tmpSortBuffer(256), - _documentIdAttributeBacking(new search::SingleStringExtAttribute("[docid]") ), - _rankAttributeBacking(new search::SingleFloatExtAttribute("[rank]") ), + _documentIdAttributeBacking(std::make_shared<search::SingleStringExtAttribute>("[docid]") ), + _rankAttributeBacking(std::make_shared<search::SingleFloatExtAttribute>("[rank]") ), _documentIdAttribute(dynamic_cast<search::SingleStringExtAttribute &>(*_documentIdAttributeBacking)), _rankAttribute(dynamic_cast<search::SingleFloatExtAttribute &>(*_rankAttributeBacking)), _shouldFillRankAttribute(false), @@ -680,8 +680,8 @@ SearchVisitor::setupScratchDocument(const StringFieldIdTMap & fieldsInQuery) void SearchVisitor::setupDocsumObjects() { - std::unique_ptr<DocsumFilter> docsumFilter(new DocsumFilter(_vsmAdapter->getDocsumTools(), - _rankController.getRankProcessor()->getHitCollector())); + auto docsumFilter = std::make_unique<DocsumFilter>(_vsmAdapter->getDocsumTools(), + _rankController.getRankProcessor()->getHitCollector()); docsumFilter->init(_fieldSearchSpecMap.nameIdMap(), *_fieldPathMap); docsumFilter->setSnippetModifiers(_snippetModifierManager.getModifiers()); _summaryGenerator.setFilter(std::move(docsumFilter)); @@ -815,7 +815,7 @@ SearchVisitor::setupGrouping(const std::vector<char> & groupingBlob) uint32_t numGroupings(0); is >> numGroupings; for(size_t i(0); i < numGroupings; i++) { - std::unique_ptr<Grouping> ag(new Grouping()); + auto ag = std::make_unique<Grouping>(); ag->deserialize(is); GroupingList::value_type groupingPtr(ag.release()); Grouping & grouping = *groupingPtr; @@ -882,7 +882,7 @@ SearchVisitor::handleDocuments(const document::BucketId&, const document::DocumentType* defaultDocType = _docTypeMapping.getDefaultDocumentType(); assert(defaultDocType); for (const auto & entry : entries) { - StorageDocument::UP document(new StorageDocument(entry->releaseDocument(), _fieldPathMap, highestFieldNo)); + auto document = std::make_unique<StorageDocument>(entry->releaseDocument(), _fieldPathMap, highestFieldNo); try { if (defaultDocType != nullptr |