diff options
121 files changed, 1648 insertions, 787 deletions
diff --git a/client/go/internal/cli/auth/auth0/auth0.go b/client/go/internal/cli/auth/auth0/auth0.go index 5f7612d4d2e..6fcd3f7680e 100644 --- a/client/go/internal/cli/auth/auth0/auth0.go +++ b/client/go/internal/cli/auth/auth0/auth0.go @@ -110,28 +110,40 @@ func (a *Client) getDeviceFlowConfig() (flowConfig, error) { } r, err := a.httpClient.Do(req, time.Second*30) if err != nil { - return flowConfig{}, fmt.Errorf("failed to get device flow config: %w", err) + return flowConfig{}, fmt.Errorf("auth0: failed to get device flow config: %w", err) } defer r.Body.Close() if r.StatusCode/100 != 2 { - return flowConfig{}, fmt.Errorf("failed to get device flow config: got response code %d from %s", r.StatusCode, url) + return flowConfig{}, fmt.Errorf("auth0: failed to get device flow config: got response code %d from %s", r.StatusCode, url) } var cfg flowConfig if err := json.NewDecoder(r.Body).Decode(&cfg); err != nil { - return flowConfig{}, fmt.Errorf("failed to decode response: %w", err) + return flowConfig{}, fmt.Errorf("auth0: failed to decode response: %w", err) } return cfg, nil } +func (a *Client) Authenticate(request *http.Request) error { + accessToken, err := a.AccessToken() + if err != nil { + return err + } + if request.Header == nil { + request.Header = make(http.Header) + } + request.Header.Set("Authorization", "Bearer "+accessToken) + return nil +} + // AccessToken returns an access token for the configured system, refreshing it if necessary. func (a *Client) AccessToken() (string, error) { creds, ok := a.provider.Systems[a.options.SystemName] if !ok { - return "", fmt.Errorf("system %s is not configured", a.options.SystemName) + return "", fmt.Errorf("auth0: system %s is not configured: %s", a.options.SystemName, reauthMessage) } else if creds.AccessToken == "" { - return "", fmt.Errorf("access token missing: %s", reauthMessage) + return "", fmt.Errorf("auth0: access token missing: %s", reauthMessage) } else if scopesChanged(creds) { - return "", fmt.Errorf("authentication scopes changed: %s", reauthMessage) + return "", fmt.Errorf("auth0: authentication scopes changed: %s", reauthMessage) } else if isExpired(creds.ExpiresAt, accessTokenExpiry) { // check if the stored access token is expired: // use the refresh token to get a new access token: @@ -142,7 +154,7 @@ func (a *Client) AccessToken() (string, error) { } resp, err := tr.Refresh(cancelOnInterrupt(), a.options.SystemName) if err != nil { - return "", fmt.Errorf("failed to renew access token: %w: %s", err, reauthMessage) + return "", fmt.Errorf("auth0: failed to renew access token: %w: %s", err, reauthMessage) } else { // persist the updated system with renewed access token creds.AccessToken = resp.AccessToken @@ -173,12 +185,6 @@ func scopesChanged(s Credentials) bool { return false } -// HasCredentials returns true if this client has retrived credentials for the configured system. -func (a *Client) HasCredentials() bool { - _, ok := a.provider.Systems[a.options.SystemName] - return ok -} - // WriteCredentials writes given credentials to the configuration file. func (a *Client) WriteCredentials(credentials Credentials) error { if a.provider.Systems == nil { @@ -186,7 +192,7 @@ func (a *Client) WriteCredentials(credentials Credentials) error { } a.provider.Systems[a.options.SystemName] = credentials if err := writeConfig(a.provider, a.options.ConfigPath); err != nil { - return fmt.Errorf("failed to write config: %w", err) + return fmt.Errorf("auth0: failed to write config: %w", err) } return nil } @@ -195,11 +201,11 @@ func (a *Client) WriteCredentials(credentials Credentials) error { func (a *Client) RemoveCredentials() error { tr := &auth.TokenRetriever{Secrets: &auth.Keyring{}} if err := tr.Delete(a.options.SystemName); err != nil { - return fmt.Errorf("failed to remove system %s from secret storage: %w", a.options.SystemName, err) + return fmt.Errorf("auth0: failed to remove system %s from secret storage: %w", a.options.SystemName, err) } delete(a.provider.Systems, a.options.SystemName) if err := writeConfig(a.provider, a.options.ConfigPath); err != nil { - return fmt.Errorf("failed to write config: %w", err) + return fmt.Errorf("auth0: failed to write config: %w", err) } return nil } diff --git a/client/go/internal/cli/auth/zts/zts.go b/client/go/internal/cli/auth/zts/zts.go index caa2d03367d..2c66ff13e8b 100644 --- a/client/go/internal/cli/auth/zts/zts.go +++ b/client/go/internal/cli/auth/zts/zts.go @@ -1,7 +1,6 @@ package zts import ( - "crypto/tls" "encoding/json" "fmt" "net/http" @@ -18,26 +17,39 @@ const DefaultURL = "https://zts.athenz.ouroath.com:4443" type Client struct { client util.HTTPClient tokenURL *url.URL + domain string } // NewClient creates a new client for an Athenz ZTS service located at serviceURL. -func NewClient(client util.HTTPClient, serviceURL string) (*Client, error) { +func NewClient(client util.HTTPClient, domain, serviceURL string) (*Client, error) { tokenURL, err := url.Parse(serviceURL) if err != nil { return nil, err } tokenURL.Path = "/zts/v1/oauth2/token" - return &Client{tokenURL: tokenURL, client: client}, nil + return &Client{tokenURL: tokenURL, client: client, domain: domain}, nil } -// AccessToken returns an access token within the given domain, using certificate to authenticate with ZTS. -func (c *Client) AccessToken(domain string, certificate tls.Certificate) (string, error) { - data := fmt.Sprintf("grant_type=client_credentials&scope=%s:domain", domain) +func (c *Client) Authenticate(request *http.Request) error { + accessToken, err := c.AccessToken() + if err != nil { + return err + } + if request.Header == nil { + request.Header = make(http.Header) + } + request.Header.Add("Authorization", "Bearer "+accessToken) + return nil +} + +// AccessToken returns an access token within the domain configured in client c. +func (c *Client) AccessToken() (string, error) { + // TODO(mpolden): This should cache and re-use tokens until expiry + data := fmt.Sprintf("grant_type=client_credentials&scope=%s:domain", c.domain) req, err := http.NewRequest("POST", c.tokenURL.String(), strings.NewReader(data)) if err != nil { return "", err } - util.SetCertificates(c.client, []tls.Certificate{certificate}) response, err := c.client.Do(req, 10*time.Second) if err != nil { return "", err @@ -45,7 +57,7 @@ func (c *Client) AccessToken(domain string, certificate tls.Certificate) (string defer response.Body.Close() if response.StatusCode != http.StatusOK { - return "", fmt.Errorf("got status %d from %s", response.StatusCode, c.tokenURL.String()) + return "", fmt.Errorf("zts: got status %d from %s", response.StatusCode, c.tokenURL.String()) } var ztsResponse struct { AccessToken string `json:"access_token"` diff --git a/client/go/internal/cli/auth/zts/zts_test.go b/client/go/internal/cli/auth/zts/zts_test.go index d0cc7ea9f9d..1c75a94ee03 100644 --- a/client/go/internal/cli/auth/zts/zts_test.go +++ b/client/go/internal/cli/auth/zts/zts_test.go @@ -1,7 +1,6 @@ package zts import ( - "crypto/tls" "testing" "github.com/vespa-engine/vespa/client/go/internal/mock" @@ -9,17 +8,17 @@ import ( func TestAccessToken(t *testing.T) { httpClient := mock.HTTPClient{} - client, err := NewClient(&httpClient, "http://example.com") + client, err := NewClient(&httpClient, "vespa.vespa", "http://example.com") if err != nil { t.Fatal(err) } httpClient.NextResponseString(400, `{"message": "bad request"}`) - _, err = client.AccessToken("vespa.vespa", tls.Certificate{}) + _, err = client.AccessToken() if err == nil { t.Fatal("want error for non-ok response status") } httpClient.NextResponseString(200, `{"access_token": "foo bar"}`) - token, err := client.AccessToken("vespa.vespa", tls.Certificate{}) + token, err := client.AccessToken() if err != nil { t.Fatal(err) } diff --git a/client/go/internal/cli/cmd/cert.go b/client/go/internal/cli/cmd/cert.go index 7f79a9db358..48bad974c3f 100644 --- a/client/go/internal/cli/cmd/cert.go +++ b/client/go/internal/cli/cmd/cert.go @@ -34,13 +34,18 @@ package specified as an argument to this command (default '.'). It's possible to override the private key and certificate used through environment variables. This can be useful in continuous integration systems. -Example of setting the certificate and key in-line: +It's also possible override the CA certificate which can be useful when using self-signed certificates with a +self-hosted Vespa service. See https://docs.vespa.ai/en/mtls.html for more information. +Example of setting the CA certificate, certificate and key in-line: + + export VESPA_CLI_DATA_PLANE_CA_CERT="my CA cert" export VESPA_CLI_DATA_PLANE_CERT="my cert" export VESPA_CLI_DATA_PLANE_KEY="my private key" -Example of loading certificate and key from custom paths: +Example of loading CA certificate, certificate and key from custom paths: + export VESPA_CLI_DATA_PLANE_CA_CERT_FILE=/path/to/cacert export VESPA_CLI_DATA_PLANE_CERT_FILE=/path/to/cert export VESPA_CLI_DATA_PLANE_KEY_FILE=/path/to/key diff --git a/client/go/internal/cli/cmd/config.go b/client/go/internal/cli/cmd/config.go index 2d32c454842..e2132814386 100644 --- a/client/go/internal/cli/cmd/config.go +++ b/client/go/internal/cli/cmd/config.go @@ -19,7 +19,6 @@ import ( "github.com/fatih/color" "github.com/spf13/cobra" "github.com/spf13/pflag" - "github.com/vespa-engine/vespa/client/go/internal/cli/auth/auth0" "github.com/vespa-engine/vespa/client/go/internal/cli/config" "github.com/vespa-engine/vespa/client/go/internal/vespa" ) @@ -250,9 +249,10 @@ type Config struct { } type KeyPair struct { - KeyPair tls.Certificate - CertificateFile string - PrivateKeyFile string + KeyPair tls.Certificate + RootCertificates []byte + CertificateFile string + PrivateKeyFile string } func loadConfig(environment map[string]string, flags map[string]*pflag.Flag) (*Config, error) { @@ -392,6 +392,10 @@ func (c *Config) deploymentIn(system vespa.System) (vespa.Deployment, error) { return vespa.Deployment{System: system, Application: app, Zone: zone}, nil } +func (c *Config) caCertificatePath() string { + return c.environment["VESPA_CLI_DATA_PLANE_CA_CERT_FILE"] +} + func (c *Config) certificatePath(app vespa.ApplicationID, targetType string) (string, error) { if override, ok := c.environment["VESPA_CLI_DATA_PLANE_CERT_FILE"]; ok { return override, nil @@ -412,50 +416,68 @@ func (c *Config) privateKeyPath(app vespa.ApplicationID, targetType string) (str return c.applicationFilePath(app, "data-plane-private-key.pem") } -func (c *Config) x509KeyPair(app vespa.ApplicationID, targetType string) (KeyPair, error) { +func (c *Config) readTLSOptions(app vespa.ApplicationID, targetType string) (vespa.TLSOptions, error) { + _, trustAll := c.environment["VESPA_CLI_DATA_PLANE_TRUST_ALL"] cert, certOk := c.environment["VESPA_CLI_DATA_PLANE_CERT"] key, keyOk := c.environment["VESPA_CLI_DATA_PLANE_KEY"] - var ( - kp tls.Certificate - err error - certFile string - keyFile string - ) + caCertText, caCertOk := c.environment["VESPA_CLI_DATA_PLANE_CA_CERT"] + options := vespa.TLSOptions{TrustAll: trustAll} + // CA certificate + if caCertOk { + options.CACertificate = []byte(caCertText) + } else { + caCertFile := c.caCertificatePath() + if caCertFile != "" { + b, err := os.ReadFile(caCertFile) + if err != nil { + return options, err + } + options.CACertificate = b + options.CACertificateFile = caCertFile + } + } + // Certificate and private key if certOk && keyOk { - // Use key pair from environment - kp, err = tls.X509KeyPair([]byte(cert), []byte(key)) + kp, err := tls.X509KeyPair([]byte(cert), []byte(key)) + if err != nil { + return vespa.TLSOptions{}, err + } + options.KeyPair = []tls.Certificate{kp} } else { - keyFile, err = c.privateKeyPath(app, targetType) + keyFile, err := c.privateKeyPath(app, targetType) if err != nil { - return KeyPair{}, err + return vespa.TLSOptions{}, err } - certFile, err = c.certificatePath(app, targetType) + certFile, err := c.certificatePath(app, targetType) if err != nil { - return KeyPair{}, err + return vespa.TLSOptions{}, err + } + kp, err := tls.LoadX509KeyPair(certFile, keyFile) + if err == nil { + options.KeyPair = []tls.Certificate{kp} + options.PrivateKeyFile = keyFile + options.CertificateFile = certFile + } else if err != nil && !os.IsNotExist(err) { + return vespa.TLSOptions{}, err } - kp, err = tls.LoadX509KeyPair(certFile, keyFile) - } - if err != nil { - return KeyPair{}, err } - if targetType == vespa.TargetHosted { - cert, err := x509.ParseCertificate(kp.Certificate[0]) + if options.KeyPair != nil { + cert, err := x509.ParseCertificate(options.KeyPair[0].Certificate[0]) if err != nil { - return KeyPair{}, err + return vespa.TLSOptions{}, err } now := time.Now() expiredAt := cert.NotAfter if expiredAt.Before(now) { delta := now.Sub(expiredAt).Truncate(time.Second) - return KeyPair{}, fmt.Errorf("certificate %s expired at %s (%s ago)", certFile, cert.NotAfter, delta) + source := options.CertificateFile + if source == "" { + source = "environment" + } + return vespa.TLSOptions{}, fmt.Errorf("certificate in %s expired at %s (%s ago)", source, cert.NotAfter, delta) } - return KeyPair{KeyPair: kp, CertificateFile: certFile, PrivateKeyFile: keyFile}, nil } - return KeyPair{ - KeyPair: kp, - CertificateFile: certFile, - PrivateKeyFile: keyFile, - }, nil + return options, nil } func (c *Config) apiKeyFileFromEnv() (string, bool) { @@ -490,11 +512,10 @@ func (c *Config) readAPIKey(cli *CLI, system vespa.System, tenantName string) ([ return nil, nil // Vespa Cloud CI only talks to data plane and does not have an API key } if !cli.isCI() { - client, err := cli.auth0Factory(cli.httpClient, auth0.Options{ConfigPath: c.authConfigPath(), SystemName: system.Name, SystemURL: system.URL}) - if err == nil && client.HasCredentials() { - return nil, nil // use Auth0 + if _, err := os.Stat(c.authConfigPath()); err == nil { + return nil, nil // We have auth config, so we should prefer Auth0 over API key } - cli.printWarning("Authenticating with API key. This is discouraged in non-CI environments", "Authenticate with 'vespa auth login'") + cli.printWarning("Authenticating with API key. This is discouraged in non-CI environments", "Authenticate with 'vespa auth login' instead") } return os.ReadFile(c.apiKeyPath(tenantName)) } diff --git a/client/go/internal/cli/cmd/config_test.go b/client/go/internal/cli/cmd/config_test.go index 458878b4356..66b65bf402b 100644 --- a/client/go/internal/cli/cmd/config_test.go +++ b/client/go/internal/cli/cmd/config_test.go @@ -2,15 +2,21 @@ package cmd import ( + "crypto/rand" + "crypto/rsa" + "crypto/tls" + "crypto/x509" + "crypto/x509/pkix" + "encoding/pem" + "math/big" "os" "path/filepath" "testing" + "time" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" - "github.com/vespa-engine/vespa/client/go/internal/cli/auth/auth0" "github.com/vespa-engine/vespa/client/go/internal/mock" - "github.com/vespa-engine/vespa/client/go/internal/util" "github.com/vespa-engine/vespa/client/go/internal/vespa" ) @@ -166,7 +172,7 @@ func TestReadAPIKey(t *testing.T) { require.Nil(t, err) assert.Equal(t, []byte("foo"), key) - // Cloud CI does not read key from disk as it's not expected to have any + // Cloud CI never reads key from disk as it's not expected to have any cli, _, _ = newTestCLI(t, "VESPA_CLI_CLOUD_CI=true") key, err = cli.config.readAPIKey(cli, vespa.PublicSystem, "t1") require.Nil(t, err) @@ -186,12 +192,111 @@ func TestReadAPIKey(t *testing.T) { require.Nil(t, err) assert.Equal(t, []byte("baz"), key) - // Auth0 is preferred when configured + // Prefer Auth0 if we have auth config cli, _, _ = newTestCLI(t) - cli.auth0Factory = func(httpClient util.HTTPClient, options auth0.Options) (auth0Client, error) { - return &mockAuth0{hasCredentials: true}, nil - } + require.Nil(t, os.WriteFile(filepath.Join(cli.config.homeDir, "auth.json"), []byte("foo"), 0600)) key, err = cli.config.readAPIKey(cli, vespa.PublicSystem, "t1") require.Nil(t, err) assert.Nil(t, key) } + +func TestConfigReadTLSOptions(t *testing.T) { + app := vespa.ApplicationID{Tenant: "t1", Application: "a1", Instance: "i1"} + homeDir := t.TempDir() + + // No environment variables, and no files on disk + assertTLSOptions(t, homeDir, app, vespa.TargetLocal, vespa.TLSOptions{}) + + // A single environment variable is set + assertTLSOptions(t, homeDir, app, vespa.TargetLocal, vespa.TLSOptions{TrustAll: true}, "VESPA_CLI_DATA_PLANE_TRUST_ALL=true") + + // Key pair is provided in-line in environment variables + pemCert, pemKey, keyPair := createKeyPair(t) + assertTLSOptions(t, homeDir, app, + vespa.TargetLocal, + vespa.TLSOptions{ + TrustAll: true, + CACertificate: []byte("cacert"), + KeyPair: []tls.Certificate{keyPair}, + }, + "VESPA_CLI_DATA_PLANE_TRUST_ALL=true", + "VESPA_CLI_DATA_PLANE_CA_CERT=cacert", + "VESPA_CLI_DATA_PLANE_CERT="+string(pemCert), + "VESPA_CLI_DATA_PLANE_KEY="+string(pemKey), + ) + + // Key pair is provided as file paths through environment variables + certFile := filepath.Join(homeDir, "cert") + keyFile := filepath.Join(homeDir, "key") + caCertFile := filepath.Join(homeDir, "cacert") + require.Nil(t, os.WriteFile(certFile, pemCert, 0600)) + require.Nil(t, os.WriteFile(keyFile, pemKey, 0600)) + require.Nil(t, os.WriteFile(caCertFile, []byte("cacert"), 0600)) + assertTLSOptions(t, homeDir, app, + vespa.TargetLocal, + vespa.TLSOptions{ + KeyPair: []tls.Certificate{keyPair}, + CACertificate: []byte("cacert"), + CACertificateFile: caCertFile, + CertificateFile: certFile, + PrivateKeyFile: keyFile, + }, + "VESPA_CLI_DATA_PLANE_CERT_FILE="+certFile, + "VESPA_CLI_DATA_PLANE_KEY_FILE="+keyFile, + "VESPA_CLI_DATA_PLANE_CA_CERT_FILE="+caCertFile, + ) + + // Key pair resides in default paths + defaultCertFile := filepath.Join(homeDir, app.String(), "data-plane-public-cert.pem") + defaultKeyFile := filepath.Join(homeDir, app.String(), "data-plane-private-key.pem") + require.Nil(t, os.WriteFile(defaultCertFile, pemCert, 0600)) + require.Nil(t, os.WriteFile(defaultKeyFile, pemKey, 0600)) + assertTLSOptions(t, homeDir, app, + vespa.TargetLocal, + vespa.TLSOptions{ + KeyPair: []tls.Certificate{keyPair}, + CertificateFile: defaultCertFile, + PrivateKeyFile: defaultKeyFile, + }, + ) +} + +func assertTLSOptions(t *testing.T, homeDir string, app vespa.ApplicationID, target string, want vespa.TLSOptions, envVars ...string) { + t.Helper() + envVars = append(envVars, "VESPA_CLI_HOME="+homeDir) + cli, _, _ := newTestCLI(t, envVars...) + require.Nil(t, cli.Run("config", "set", "application", app.String())) + config, err := cli.config.readTLSOptions(app, vespa.TargetLocal) + require.Nil(t, err) + assert.Equal(t, want, config) +} + +func createKeyPair(t *testing.T) ([]byte, []byte, tls.Certificate) { + privateKey, err := rsa.GenerateKey(rand.Reader, 2048) + if err != nil { + t.Fatal(err) + } + notBefore := time.Now() + notAfter := notBefore.Add(24 * time.Hour) + template := x509.Certificate{ + SerialNumber: big.NewInt(1), + Subject: pkix.Name{CommonName: "example.com"}, + NotBefore: notBefore, + NotAfter: notAfter, + } + certificateDER, err := x509.CreateCertificate(rand.Reader, &template, &template, &privateKey.PublicKey, privateKey) + if err != nil { + t.Fatal(err) + } + privateKeyDER, err := x509.MarshalPKCS8PrivateKey(privateKey) + if err != nil { + t.Fatal(err) + } + pemCert := pem.EncodeToMemory(&pem.Block{Type: "CERTIFICATE", Bytes: certificateDER}) + pemKey := pem.EncodeToMemory(&pem.Block{Type: "PRIVATE KEY", Bytes: privateKeyDER}) + kp, err := tls.X509KeyPair(pemCert, pemKey) + if err != nil { + t.Fatal(err) + } + return pemCert, pemKey, kp +} diff --git a/client/go/internal/cli/cmd/curl.go b/client/go/internal/cli/cmd/curl.go index 8fcd1fa6ef7..3d5aaff24dc 100644 --- a/client/go/internal/cli/cmd/curl.go +++ b/client/go/internal/cli/cmd/curl.go @@ -4,7 +4,6 @@ package cmd import ( "fmt" "log" - "net/http" "os" "strings" @@ -54,6 +53,7 @@ $ vespa curl -- -v --data-urlencode "yql=select * from music where album contain return err } case vespa.DocumentService, vespa.QueryService: + c.CaCertificate = service.TLSOptions.CACertificateFile c.PrivateKey = service.TLSOptions.PrivateKeyFile c.Certificate = service.TLSOptions.CertificateFile default: @@ -79,15 +79,7 @@ func addAccessToken(cmd *curl.Command, target vespa.Target) error { if target.Type() != vespa.TargetCloud { return nil } - req := http.Request{} - if err := target.SignRequest(&req, ""); err != nil { - return err - } - headerValue := req.Header.Get("Authorization") - if headerValue == "" { - return fmt.Errorf("no authorization header added when signing request") - } - cmd.Header("Authorization", headerValue) + cmd.Header("Authorization", "secret") return nil } diff --git a/client/go/internal/cli/cmd/feed.go b/client/go/internal/cli/cmd/feed.go index 895a22d2be5..f0f82dd80d1 100644 --- a/client/go/internal/cli/cmd/feed.go +++ b/client/go/internal/cli/cmd/feed.go @@ -14,16 +14,24 @@ import ( "github.com/vespa-engine/vespa/client/go/internal/vespa/document" ) -func addFeedFlags(cmd *cobra.Command, verbose *bool, connections *int) { - cmd.PersistentFlags().IntVarP(connections, "connections", "N", 8, "The number of connections to use") - cmd.PersistentFlags().BoolVarP(verbose, "verbose", "v", false, "Verbose mode. Print errors as they happen") +func addFeedFlags(cmd *cobra.Command, options *feedOptions) { + cmd.PersistentFlags().IntVar(&options.connections, "connections", 8, "The number of connections to use") + cmd.PersistentFlags().StringVar(&options.route, "route", "", "Target Vespa route for feed operations") + cmd.PersistentFlags().IntVar(&options.traceLevel, "trace", 0, "The trace level of network traffic. 0 to disable") + cmd.PersistentFlags().IntVar(&options.timeoutSecs, "timeout", 0, "Feed operation timeout in seconds. 0 to disable") + cmd.PersistentFlags().BoolVar(&options.verbose, "verbose", false, "Verbose mode. Print successful operations in addition to errors") +} + +type feedOptions struct { + connections int + route string + verbose bool + traceLevel int + timeoutSecs int } func newFeedCmd(cli *CLI) *cobra.Command { - var ( - verbose bool - connections int - ) + var options feedOptions cmd := &cobra.Command{ Use: "feed FILE", Short: "Feed documents to a Vespa cluster", @@ -56,10 +64,10 @@ $ cat documents.jsonl | vespa feed - defer f.Close() r = f } - return feed(r, cli, verbose, connections) + return feed(r, cli, options) }, } - addFeedFlags(cmd, &verbose, &connections) + addFeedFlags(cmd, &options) return cmd } @@ -67,29 +75,29 @@ func createServiceClients(service *vespa.Service, n int) []util.HTTPClient { clients := make([]util.HTTPClient, 0, n) for i := 0; i < n; i++ { client := service.Client().Clone() - util.ForceHTTP2(client, service.TLSOptions.KeyPair) // Feeding should always use HTTP/2 + // Feeding should always use HTTP/2 + util.ForceHTTP2(client, service.TLSOptions.KeyPair, service.TLSOptions.CACertificate, service.TLSOptions.TrustAll) clients = append(clients, client) } return clients } -func feed(r io.Reader, cli *CLI, verbose bool, connections int) error { +func feed(r io.Reader, cli *CLI, options feedOptions) error { service, err := documentService(cli) if err != nil { return err } - clients := createServiceClients(service, connections) + clients := createServiceClients(service, options.connections) client := document.NewClient(document.ClientOptions{ - BaseURL: service.BaseURL, + Timeout: time.Duration(options.timeoutSecs) * time.Second, + Route: options.route, + TraceLevel: options.traceLevel, + BaseURL: service.BaseURL, }, clients) - throttler := document.NewThrottler(connections) + throttler := document.NewThrottler(options.connections) // TODO(mpolden): Make doom duration configurable circuitBreaker := document.NewCircuitBreaker(10*time.Second, 0) - errWriter := io.Discard - if verbose { - errWriter = cli.Stderr - } - dispatcher := document.NewDispatcher(client, throttler, circuitBreaker, errWriter) + dispatcher := document.NewDispatcher(client, throttler, circuitBreaker, cli.Stderr, options.verbose) dec := document.NewDecoder(r) start := cli.now() diff --git a/client/go/internal/cli/cmd/root.go b/client/go/internal/cli/cmd/root.go index 360af9d0dcf..88d43411983 100644 --- a/client/go/internal/cli/cmd/root.go +++ b/client/go/internal/cli/cmd/root.go @@ -2,7 +2,6 @@ package cmd import ( - "crypto/tls" "encoding/json" "fmt" "io" @@ -88,18 +87,9 @@ func (c *execSubprocess) Run(name string, args ...string) ([]byte, error) { return exec.Command(name, args...).Output() } -type ztsClient interface { - AccessToken(domain string, certficiate tls.Certificate) (string, error) -} - -type auth0Client interface { - AccessToken() (string, error) - HasCredentials() bool -} - -type auth0Factory func(httpClient util.HTTPClient, options auth0.Options) (auth0Client, error) +type auth0Factory func(httpClient util.HTTPClient, options auth0.Options) (vespa.Authenticator, error) -type ztsFactory func(httpClient util.HTTPClient, url string) (ztsClient, error) +type ztsFactory func(httpClient util.HTTPClient, domain, url string) (vespa.Authenticator, error) // New creates the Vespa CLI, writing output to stdout and stderr, and reading environment variables from environment. func New(stdout, stderr io.Writer, environment []string) (*CLI, error) { @@ -143,11 +133,11 @@ For detailed description of flags and configuration, see 'vespa help config'. httpClient: util.CreateClient(time.Second * 10), exec: &execSubprocess{}, now: time.Now, - auth0Factory: func(httpClient util.HTTPClient, options auth0.Options) (auth0Client, error) { + auth0Factory: func(httpClient util.HTTPClient, options auth0.Options) (vespa.Authenticator, error) { return auth0.NewClient(httpClient, options) }, - ztsFactory: func(httpClient util.HTTPClient, url string) (ztsClient, error) { - return zts.NewClient(httpClient, url) + ztsFactory: func(httpClient util.HTTPClient, domain, url string) (vespa.Authenticator, error) { + return zts.NewClient(httpClient, domain, url) }, } cli.isTerminal = func() bool { return isTerminal(cli.Stdout) && isTerminal(cli.Stderr) } @@ -321,16 +311,34 @@ func (c *CLI) createTarget(opts targetOptions) (vespa.Target, error) { if err != nil { return nil, err } + customURL := "" if strings.HasPrefix(targetType, "http") { - return vespa.CustomTarget(c.httpClient, targetType), nil + customURL = targetType + targetType = vespa.TargetCustom } switch targetType { - case vespa.TargetLocal: - return vespa.LocalTarget(c.httpClient), nil + case vespa.TargetLocal, vespa.TargetCustom: + return c.createCustomTarget(targetType, customURL) case vespa.TargetCloud, vespa.TargetHosted: return c.createCloudTarget(targetType, opts) + default: + return nil, errHint(fmt.Errorf("invalid target: %s", targetType), "Valid targets are 'local', 'cloud', 'hosted' or an URL") + } +} + +func (c *CLI) createCustomTarget(targetType, customURL string) (vespa.Target, error) { + tlsOptions, err := c.config.readTLSOptions(vespa.DefaultApplication, targetType) + if err != nil { + return nil, err + } + switch targetType { + case vespa.TargetLocal: + return vespa.LocalTarget(c.httpClient, tlsOptions), nil + case vespa.TargetCustom: + return vespa.CustomTarget(c.httpClient, customURL, tlsOptions), nil + default: + return nil, fmt.Errorf("invalid custom target: %s", targetType) } - return nil, errHint(fmt.Errorf("invalid target: %s", targetType), "Valid targets are 'local', 'cloud', 'hosted' or an URL") } func (c *CLI) createCloudTarget(targetType string, opts targetOptions) (vespa.Target, error) { @@ -347,48 +355,53 @@ func (c *CLI) createCloudTarget(targetType string, opts targetOptions) (vespa.Ta return nil, err } var ( - apiKey []byte - authConfigPath string + apiAuth vespa.Authenticator + deploymentAuth vespa.Authenticator apiTLSOptions vespa.TLSOptions deploymentTLSOptions vespa.TLSOptions ) switch targetType { case vespa.TargetCloud: - apiKey, err = c.config.readAPIKey(c, system, deployment.Application.Tenant) + apiKey, err := c.config.readAPIKey(c, system, deployment.Application.Tenant) if err != nil { return nil, err } - authConfigPath = c.config.authConfigPath() + if apiKey == nil { + authConfigPath := c.config.authConfigPath() + auth0, err := c.auth0Factory(c.httpClient, auth0.Options{ConfigPath: authConfigPath, SystemName: system.Name, SystemURL: system.URL}) + if err != nil { + return nil, err + } + apiAuth = auth0 + } else { + apiAuth = vespa.NewRequestSigner(deployment.Application.SerializedForm(), apiKey) + } deploymentTLSOptions = vespa.TLSOptions{} if !opts.noCertificate { - kp, err := c.config.x509KeyPair(deployment.Application, targetType) + kp, err := c.config.readTLSOptions(deployment.Application, targetType) if err != nil { - return nil, errHint(err, "Deployment to cloud requires a certificate. Try 'vespa auth cert'") - } - deploymentTLSOptions = vespa.TLSOptions{ - KeyPair: []tls.Certificate{kp.KeyPair}, - CertificateFile: kp.CertificateFile, - PrivateKeyFile: kp.PrivateKeyFile, + return nil, errHint(err, "Deployment to cloud requires a certificate", "Try 'vespa auth cert' to create a self-signed certificate") } + deploymentTLSOptions = kp } case vespa.TargetHosted: - kp, err := c.config.x509KeyPair(deployment.Application, targetType) + kp, err := c.config.readTLSOptions(deployment.Application, targetType) if err != nil { return nil, errHint(err, "Deployment to hosted requires an Athenz certificate", "Try renewing certificate with 'athenz-user-cert'") } - apiTLSOptions = vespa.TLSOptions{ - KeyPair: []tls.Certificate{kp.KeyPair}, - CertificateFile: kp.CertificateFile, - PrivateKeyFile: kp.PrivateKeyFile, + zts, err := c.ztsFactory(c.httpClient, system.AthenzDomain, zts.DefaultURL) + if err != nil { + return nil, err } - deploymentTLSOptions = apiTLSOptions + deploymentAuth = zts + apiTLSOptions = kp + deploymentTLSOptions = kp default: return nil, fmt.Errorf("invalid cloud target: %s", targetType) } apiOptions := vespa.APIOptions{ System: system, TLSOptions: apiTLSOptions, - APIKey: apiKey, } deploymentOptions := vespa.CloudDeploymentOptions{ Deployment: deployment, @@ -403,15 +416,7 @@ func (c *CLI) createCloudTarget(targetType string, opts targetOptions) (vespa.Ta Writer: c.Stdout, Level: vespa.LogLevel(logLevel), } - auth0, err := c.auth0Factory(c.httpClient, auth0.Options{ConfigPath: authConfigPath, SystemName: apiOptions.System.Name, SystemURL: apiOptions.System.URL}) - if err != nil { - return nil, err - } - zts, err := c.ztsFactory(c.httpClient, zts.DefaultURL) - if err != nil { - return nil, err - } - return vespa.CloudTarget(c.httpClient, zts, auth0, apiOptions, deploymentOptions, logOptions) + return vespa.CloudTarget(c.httpClient, apiAuth, deploymentAuth, apiOptions, deploymentOptions, logOptions) } // system returns the appropiate system for the target configured in this CLI. @@ -460,7 +465,6 @@ func (c *CLI) createDeploymentOptions(pkg vespa.ApplicationPackage, target vespa ApplicationPackage: pkg, Target: target, Timeout: timeout, - HTTPClient: c.httpClient, }, nil } diff --git a/client/go/internal/cli/cmd/test.go b/client/go/internal/cli/cmd/test.go index 05633b1135e..8c4501e2870 100644 --- a/client/go/internal/cli/cmd/test.go +++ b/client/go/internal/cli/cmd/test.go @@ -263,7 +263,7 @@ func verify(step step, defaultCluster string, defaultParameters map[string]strin var response *http.Response if externalEndpoint { - util.SetCertificates(context.cli.httpClient, []tls.Certificate{}) + util.ConfigureTLS(context.cli.httpClient, []tls.Certificate{}, nil, false) response, err = context.cli.httpClient.Do(request, 60*time.Second) } else { response, err = service.Do(request, 600*time.Second) // Vespa should provide a response within the given request timeout diff --git a/client/go/internal/cli/cmd/testutil_test.go b/client/go/internal/cli/cmd/testutil_test.go index 61f8dab2264..492e40d8855 100644 --- a/client/go/internal/cli/cmd/testutil_test.go +++ b/client/go/internal/cli/cmd/testutil_test.go @@ -3,13 +3,14 @@ package cmd import ( "bytes" - "crypto/tls" + "net/http" "path/filepath" "testing" "github.com/vespa-engine/vespa/client/go/internal/cli/auth/auth0" "github.com/vespa-engine/vespa/client/go/internal/mock" "github.com/vespa-engine/vespa/client/go/internal/util" + "github.com/vespa-engine/vespa/client/go/internal/vespa" ) func newTestCLI(t *testing.T, envVars ...string) (*CLI, *bytes.Buffer, *bytes.Buffer) { @@ -29,21 +30,15 @@ func newTestCLI(t *testing.T, envVars ...string) (*CLI, *bytes.Buffer, *bytes.Bu httpClient := &mock.HTTPClient{} cli.httpClient = httpClient cli.exec = &mock.Exec{} - cli.auth0Factory = func(httpClient util.HTTPClient, options auth0.Options) (auth0Client, error) { - return &mockAuth0{}, nil + cli.auth0Factory = func(httpClient util.HTTPClient, options auth0.Options) (vespa.Authenticator, error) { + return &mockAuthenticator{}, nil } - cli.ztsFactory = func(httpClient util.HTTPClient, url string) (ztsClient, error) { - return &mockZTS{}, nil + cli.ztsFactory = func(httpClient util.HTTPClient, domain, url string) (vespa.Authenticator, error) { + return &mockAuthenticator{}, nil } return cli, &stdout, &stderr } -type mockZTS struct{} +type mockAuthenticator struct{} -func (z *mockZTS) AccessToken(domain string, cert tls.Certificate) (string, error) { return "", nil } - -type mockAuth0 struct{ hasCredentials bool } - -func (a *mockAuth0) AccessToken() (string, error) { return "", nil } - -func (a *mockAuth0) HasCredentials() bool { return a.hasCredentials } +func (a *mockAuthenticator) Authenticate(request *http.Request) error { return nil } diff --git a/client/go/internal/util/http.go b/client/go/internal/util/http.go index dcf05ed3a14..8a67b24dffb 100644 --- a/client/go/internal/util/http.go +++ b/client/go/internal/util/http.go @@ -4,6 +4,7 @@ package util import ( "context" "crypto/tls" + "crypto/x509" "fmt" "net" "net/http" @@ -35,7 +36,7 @@ func (c *defaultHTTPClient) Do(request *http.Request, timeout time.Duration) (re func (c *defaultHTTPClient) Clone() HTTPClient { return CreateClient(c.client.Timeout) } -func SetCertificates(client HTTPClient, certificates []tls.Certificate) { +func ConfigureTLS(client HTTPClient, certificates []tls.Certificate, caCertificate []byte, trustAll bool) { c, ok := client.(*defaultHTTPClient) if !ok { return @@ -43,8 +44,14 @@ func SetCertificates(client HTTPClient, certificates []tls.Certificate) { var tlsConfig *tls.Config = nil if certificates != nil { tlsConfig = &tls.Config{ - Certificates: certificates, - MinVersion: tls.VersionTLS12, + Certificates: certificates, + MinVersion: tls.VersionTLS12, + InsecureSkipVerify: trustAll, + } + if caCertificate != nil { + certs := x509.NewCertPool() + certs.AppendCertsFromPEM(caCertificate) + tlsConfig.RootCAs = certs } } if tr, ok := c.client.Transport.(*http.Transport); ok { @@ -56,19 +63,13 @@ func SetCertificates(client HTTPClient, certificates []tls.Certificate) { } } -func ForceHTTP2(client HTTPClient, certificates []tls.Certificate) { +func ForceHTTP2(client HTTPClient, certificates []tls.Certificate, caCertificate []byte, trustAll bool) { c, ok := client.(*defaultHTTPClient) if !ok { return } - var tlsConfig *tls.Config = nil var dialFunc func(ctx context.Context, network, addr string, cfg *tls.Config) (net.Conn, error) - if certificates != nil { - tlsConfig = &tls.Config{ - Certificates: certificates, - MinVersion: tls.VersionTLS12, - } - } else { + if certificates == nil { // No certificate, so force H2C (HTTP/2 over clear-text) by using a non-TLS Dialer dialer := net.Dialer{} dialFunc = func(ctx context.Context, network, addr string, cfg *tls.Config) (net.Conn, error) { @@ -80,10 +81,10 @@ func ForceHTTP2(client HTTPClient, certificates []tls.Certificate) { // https://github.com/golang/go/issues/16582 // https://github.com/golang/go/issues/22091 c.client.Transport = &http2.Transport{ - AllowHTTP: true, - TLSClientConfig: tlsConfig, - DialTLSContext: dialFunc, + AllowHTTP: true, + DialTLSContext: dialFunc, } + ConfigureTLS(client, certificates, caCertificate, trustAll) } func CreateClient(timeout time.Duration) HTTPClient { diff --git a/client/go/internal/vespa/crypto.go b/client/go/internal/vespa/crypto.go index 9621d0c1180..5e273538869 100644 --- a/client/go/internal/vespa/crypto.go +++ b/client/go/internal/vespa/crypto.go @@ -111,6 +111,8 @@ func NewRequestSigner(keyID string, pemPrivateKey []byte) *RequestSigner { } } +func (rs *RequestSigner) Authenticate(request *http.Request) error { return rs.SignRequest(request) } + // SignRequest signs the given HTTP request using the private key in rs func (rs *RequestSigner) SignRequest(request *http.Request) error { timestamp := rs.now().UTC().Format(time.RFC3339) diff --git a/client/go/internal/vespa/deploy.go b/client/go/internal/vespa/deploy.go index 687bfc46124..f633c8ed9ee 100644 --- a/client/go/internal/vespa/deploy.go +++ b/client/go/internal/vespa/deploy.go @@ -45,7 +45,6 @@ type DeploymentOptions struct { ApplicationPackage ApplicationPackage Timeout time.Duration Version version.Version - HTTPClient util.HTTPClient } type LogLinePrepareResponse struct { @@ -130,7 +129,7 @@ func Prepare(deployment DeploymentOptions) (PrepareResult, error) { return PrepareResult{}, err } serviceDescription := "Deploy service" - response, err := deployment.HTTPClient.Do(req, time.Second*30) + response, err := deployServiceDo(req, time.Second*30, deployment) if err != nil { return PrepareResult{}, err } @@ -171,7 +170,7 @@ func Activate(sessionID int64, deployment DeploymentOptions) error { return err } serviceDescription := "Deploy service" - response, err := deployment.HTTPClient.Do(req, time.Second*30) + response, err := deployServiceDo(req, time.Second*30, deployment) if err != nil { return err } @@ -263,11 +262,7 @@ func Submit(opts DeploymentOptions) error { } request.Header.Set("Content-Type", writer.FormDataContentType()) serviceDescription := "Submit service" - sigKeyId := opts.Target.Deployment().Application.SerializedForm() - if err := opts.Target.SignRequest(request, sigKeyId); err != nil { - return fmt.Errorf("failed to sign api request: %w", err) - } - response, err := opts.HTTPClient.Do(request, time.Minute*10) + response, err := deployServiceDo(request, time.Minute*10, opts) if err != nil { return err } @@ -275,6 +270,14 @@ func Submit(opts DeploymentOptions) error { return checkResponse(request, response, serviceDescription) } +func deployServiceDo(request *http.Request, timeout time.Duration, opts DeploymentOptions) (*http.Response, error) { + s, err := opts.Target.Service(DeployService, 0, 0, "") + if err != nil { + return nil, err + } + return s.Do(request, timeout) +} + func checkDeploymentOpts(opts DeploymentOptions) error { if opts.Target.Type() == TargetCloud && !opts.ApplicationPackage.HasCertificate() { return fmt.Errorf("%s: missing certificate in package", opts) @@ -334,11 +337,6 @@ func uploadApplicationPackage(url *url.URL, opts DeploymentOptions) (PrepareResu if err != nil { return PrepareResult{}, err } - - keyID := opts.Target.Deployment().Application.SerializedForm() - if err := opts.Target.SignRequest(request, keyID); err != nil { - return PrepareResult{}, err - } response, err := service.Do(request, time.Minute*10) if err != nil { return PrepareResult{}, err diff --git a/client/go/internal/vespa/deploy_test.go b/client/go/internal/vespa/deploy_test.go index 3e74e9ab3b6..da2604282c0 100644 --- a/client/go/internal/vespa/deploy_test.go +++ b/client/go/internal/vespa/deploy_test.go @@ -19,12 +19,11 @@ import ( func TestDeploy(t *testing.T) { httpClient := mock.HTTPClient{} - target := LocalTarget(&httpClient) + target := LocalTarget(&httpClient, TLSOptions{}) appDir, _ := mock.ApplicationPackageDir(t, false, false) opts := DeploymentOptions{ Target: target, ApplicationPackage: ApplicationPackage{Path: appDir}, - HTTPClient: &httpClient, } _, err := Deploy(opts) assert.Nil(t, err) @@ -47,7 +46,6 @@ func TestDeployCloud(t *testing.T) { opts := DeploymentOptions{ Target: target, ApplicationPackage: ApplicationPackage{Path: appDir}, - HTTPClient: &httpClient, } _, err := Deploy(opts) require.Nil(t, err) diff --git a/client/go/internal/vespa/document/dispatcher.go b/client/go/internal/vespa/document/dispatcher.go index 838a7bc45ee..533ca7a0019 100644 --- a/client/go/internal/vespa/document/dispatcher.go +++ b/client/go/internal/vespa/document/dispatcher.go @@ -4,6 +4,7 @@ import ( "container/list" "fmt" "io" + "strings" "sync" "sync/atomic" "time" @@ -18,12 +19,15 @@ type Dispatcher struct { circuitBreaker CircuitBreaker stats Stats - started bool - ready chan Id - results chan Result + started bool + ready chan Id + results chan Result + msgs chan string + inflight map[string]*documentGroup inflightCount int64 - errWriter io.Writer + output io.Writer + verbose bool mu sync.RWMutex wg sync.WaitGroup @@ -55,13 +59,14 @@ func (g *documentGroup) add(op documentOp, first bool) { } } -func NewDispatcher(feeder Feeder, throttler Throttler, breaker CircuitBreaker, errWriter io.Writer) *Dispatcher { +func NewDispatcher(feeder Feeder, throttler Throttler, breaker CircuitBreaker, output io.Writer, verbose bool) *Dispatcher { d := &Dispatcher{ feeder: feeder, throttler: throttler, circuitBreaker: breaker, inflight: make(map[string]*documentGroup), - errWriter: errWriter, + output: output, + verbose: verbose, } d.start() return d @@ -69,8 +74,6 @@ func NewDispatcher(feeder Feeder, throttler Throttler, breaker CircuitBreaker, e func (d *Dispatcher) sendDocumentIn(group *documentGroup) { group.mu.Lock() - defer group.mu.Unlock() - defer d.releaseSlot() first := group.ops.Front() if first == nil { panic("sending from empty document group, this should not happen") @@ -79,6 +82,8 @@ func (d *Dispatcher) sendDocumentIn(group *documentGroup) { op.attempts++ result := d.feeder.Send(op.document) d.results <- result + d.releaseSlot() + group.mu.Unlock() if d.shouldRetry(op, result) { d.enqueue(op) } @@ -86,29 +91,35 @@ func (d *Dispatcher) sendDocumentIn(group *documentGroup) { func (d *Dispatcher) shouldRetry(op documentOp, result Result) bool { if result.HTTPStatus/100 == 2 || result.HTTPStatus == 404 || result.HTTPStatus == 412 { + if d.verbose { + d.msgs <- fmt.Sprintf("feed: successfully fed %s with status %d", op.document.Id, result.HTTPStatus) + } d.throttler.Success() d.circuitBreaker.Success() return false } if result.HTTPStatus == 429 || result.HTTPStatus == 503 { - fmt.Fprintf(d.errWriter, "feed: %s was throttled with status %d: retrying\n", op.document, result.HTTPStatus) + d.msgs <- fmt.Sprintf("feed: %s was throttled with status %d: retrying\n", op.document, result.HTTPStatus) d.throttler.Throttled(atomic.LoadInt64(&d.inflightCount)) return true } if result.Err != nil || result.HTTPStatus == 500 || result.HTTPStatus == 502 || result.HTTPStatus == 504 { retry := op.attempts <= maxAttempts - msg := "feed: " + op.document.String() + " failed with " + var msg strings.Builder + msg.WriteString("feed: ") + msg.WriteString(op.document.String()) if result.Err != nil { - msg += "error " + result.Err.Error() + msg.WriteString("error ") + msg.WriteString(result.Err.Error()) } else { - msg += fmt.Sprintf("status %d", result.HTTPStatus) + msg.WriteString(fmt.Sprintf("status %d", result.HTTPStatus)) } if retry { - msg += ": retrying" + msg.WriteString(": retrying") } else { - msg += fmt.Sprintf(": giving up after %d attempts", maxAttempts) + msg.WriteString(fmt.Sprintf(": giving up after %d attempts", maxAttempts)) } - fmt.Fprintln(d.errWriter, msg) + d.msgs <- msg.String() d.circuitBreaker.Error(fmt.Errorf("request failed with status %d", result.HTTPStatus)) if retry { return true @@ -125,17 +136,22 @@ func (d *Dispatcher) start() { } d.ready = make(chan Id, 4096) d.results = make(chan Result, 4096) + d.msgs = make(chan string, 4096) d.started = true d.wg.Add(1) go func() { defer d.wg.Done() d.readDocuments() }() - d.resultWg.Add(1) + d.resultWg.Add(2) go func() { defer d.resultWg.Done() d.readResults() }() + go func() { + defer d.resultWg.Done() + d.readMessages() + }() } func (d *Dispatcher) readDocuments() { @@ -157,15 +173,22 @@ func (d *Dispatcher) readResults() { } } +func (d *Dispatcher) readMessages() { + for msg := range d.msgs { + fmt.Fprintln(d.output, msg) + } +} + func (d *Dispatcher) enqueue(op documentOp) error { d.mu.Lock() if !d.started { return fmt.Errorf("dispatcher is closed") } - group, ok := d.inflight[op.document.Id.String()] + key := op.document.Id.String() + group, ok := d.inflight[key] if !ok { group = &documentGroup{} - d.inflight[op.document.Id.String()] = group + d.inflight[key] = group } d.mu.Unlock() group.add(op, op.attempts > 0) @@ -188,25 +211,26 @@ func (d *Dispatcher) acquireSlot() { func (d *Dispatcher) releaseSlot() { atomic.AddInt64(&d.inflightCount, -1) } -func closeAndWait[T any](ch chan T, wg *sync.WaitGroup, d *Dispatcher, markClosed bool) { - d.mu.Lock() - if d.started { - close(ch) - if markClosed { - d.started = false - } - } - d.mu.Unlock() - wg.Wait() -} - func (d *Dispatcher) Enqueue(doc Document) error { return d.enqueue(documentOp{document: doc}) } func (d *Dispatcher) Stats() Stats { return d.stats } // Close closes the dispatcher and waits for all inflight operations to complete. func (d *Dispatcher) Close() error { - closeAndWait(d.ready, &d.wg, d, false) - closeAndWait(d.results, &d.resultWg, d, true) + d.mu.Lock() + if d.started { + close(d.ready) + } + d.mu.Unlock() + d.wg.Wait() // Wait for inflight operations to complete + + d.mu.Lock() + if d.started { + close(d.results) + close(d.msgs) + d.started = false + } + d.mu.Unlock() + d.resultWg.Wait() // Wait for results return nil } diff --git a/client/go/internal/vespa/document/dispatcher_test.go b/client/go/internal/vespa/document/dispatcher_test.go index 80bc5f603ae..d066f5bc9ae 100644 --- a/client/go/internal/vespa/document/dispatcher_test.go +++ b/client/go/internal/vespa/document/dispatcher_test.go @@ -41,7 +41,7 @@ func TestDispatcher(t *testing.T) { clock := &manualClock{tick: time.Second} throttler := newThrottler(8, clock.now) breaker := NewCircuitBreaker(time.Second, 0) - dispatcher := NewDispatcher(feeder, throttler, breaker, io.Discard) + dispatcher := NewDispatcher(feeder, throttler, breaker, io.Discard, false) docs := []Document{ {Id: mustParseId("id:ns:type::doc1"), Operation: OperationPut, Body: []byte(`{"fields":{"foo": "123"}}`)}, {Id: mustParseId("id:ns:type::doc2"), Operation: OperationPut, Body: []byte(`{"fields":{"bar": "456"}}`)}, @@ -74,7 +74,7 @@ func TestDispatcherOrdering(t *testing.T) { clock := &manualClock{tick: time.Second} throttler := newThrottler(8, clock.now) breaker := NewCircuitBreaker(time.Second, 0) - dispatcher := NewDispatcher(feeder, throttler, breaker, io.Discard) + dispatcher := NewDispatcher(feeder, throttler, breaker, io.Discard, false) for _, d := range docs { dispatcher.Enqueue(d) } @@ -110,7 +110,7 @@ func TestDispatcherOrderingWithFailures(t *testing.T) { clock := &manualClock{tick: time.Second} throttler := newThrottler(8, clock.now) breaker := NewCircuitBreaker(time.Second, 0) - dispatcher := NewDispatcher(feeder, throttler, breaker, io.Discard) + dispatcher := NewDispatcher(feeder, throttler, breaker, io.Discard, false) for _, d := range docs { dispatcher.Enqueue(d) } diff --git a/client/go/internal/vespa/document/http.go b/client/go/internal/vespa/document/http.go index 588330a0574..1bcd7eff39e 100644 --- a/client/go/internal/vespa/document/http.go +++ b/client/go/internal/vespa/document/http.go @@ -29,7 +29,7 @@ type ClientOptions struct { BaseURL string Timeout time.Duration Route string - TraceLevel *int + TraceLevel int } type countingHTTPClient struct { @@ -72,14 +72,18 @@ func NewClient(options ClientOptions, httpClients []util.HTTPClient) *Client { func (c *Client) queryParams() url.Values { params := url.Values{} - if c.options.Timeout > 0 { - params.Set("timeout", strconv.FormatInt(c.options.Timeout.Milliseconds(), 10)+"ms") + timeout := c.options.Timeout + if timeout == 0 { + timeout = 200 * time.Second + } else { + timeout = timeout*11/10 + 1000 } + params.Set("timeout", strconv.FormatInt(timeout.Milliseconds(), 10)+"ms") if c.options.Route != "" { params.Set("route", c.options.Route) } - if c.options.TraceLevel != nil { - params.Set("tracelevel", strconv.Itoa(*c.options.TraceLevel)) + if c.options.TraceLevel > 0 { + params.Set("tracelevel", strconv.Itoa(c.options.TraceLevel)) } return params } @@ -166,7 +170,7 @@ func (c *Client) Send(document Document) Result { } defer resp.Body.Close() elapsed := c.now().Sub(start) - return c.resultWithResponse(resp, result, document, elapsed) + return resultWithResponse(resp, result, document, elapsed) } func resultWithErr(result Result, err error) Result { @@ -176,7 +180,7 @@ func resultWithErr(result Result, err error) Result { return result } -func (c *Client) resultWithResponse(resp *http.Response, result Result, document Document, elapsed time.Duration) Result { +func resultWithResponse(resp *http.Response, result Result, document Document, elapsed time.Duration) Result { result.HTTPStatus = resp.StatusCode result.Stats.Responses++ result.Stats.ResponsesByCode = map[int]int64{resp.StatusCode: 1} diff --git a/client/go/internal/vespa/document/http_test.go b/client/go/internal/vespa/document/http_test.go index 43eaf1bfdf9..8f8394a5d4e 100644 --- a/client/go/internal/vespa/document/http_test.go +++ b/client/go/internal/vespa/document/http_test.go @@ -108,7 +108,7 @@ func TestClientSend(t *testing.T) { if r.Method != http.MethodPut { t.Errorf("got r.Method = %q, want %q", r.Method, http.MethodPut) } - wantURL := fmt.Sprintf("https://example.com:1337/document/v1/ns/type/docid/%s?create=true&timeout=5000ms", doc.Id.UserSpecific) + wantURL := fmt.Sprintf("https://example.com:1337/document/v1/ns/type/docid/%s?create=true&timeout=5500ms", doc.Id.UserSpecific) if r.URL.String() != wantURL { t.Errorf("got r.URL = %q, want %q", r.URL, wantURL) } diff --git a/client/go/internal/vespa/target.go b/client/go/internal/vespa/target.go index bc936623bcb..9f3fd7f5c65 100644 --- a/client/go/internal/vespa/target.go +++ b/client/go/internal/vespa/target.go @@ -7,6 +7,7 @@ import ( "fmt" "io" "net/http" + "sync" "time" "github.com/vespa-engine/vespa/client/go/internal/util" @@ -17,7 +18,7 @@ const ( // A target for a local Vespa service TargetLocal = "local" - // A target for a custom URL + // A target for a Vespa service at a custom URL TargetCustom = "custom" // A Vespa Cloud target @@ -38,13 +39,19 @@ const ( retryInterval = 2 * time.Second ) +// Authenticator authenticates the given HTTP request. +type Authenticator interface { + Authenticate(request *http.Request) error +} + // Service represents a Vespa service. type Service struct { BaseURL string Name string TLSOptions TLSOptions - zts zts + once sync.Once + auth Authenticator httpClient util.HTTPClient } @@ -65,19 +72,19 @@ type Target interface { // PrintLog writes the logs of this deployment using given options to control output. PrintLog(options LogOptions) error - // SignRequest signs request with given keyID as required by the implementation of this target. - SignRequest(request *http.Request, keyID string) error - // CheckVersion verifies whether clientVersion is compatible with this target. CheckVersion(clientVersion version.Version) error } -// TLSOptions configures the client certificate to use for cloud API or service requests. +// TLSOptions holds the client certificate to use for cloud API or service requests. type TLSOptions struct { - KeyPair []tls.Certificate - CertificateFile string - PrivateKeyFile string - AthenzDomain string + CACertificate []byte + KeyPair []tls.Certificate + TrustAll bool + + CACertificateFile string + CertificateFile string + PrivateKeyFile string } // LogOptions configures the log output to produce when writing log messages. @@ -90,17 +97,15 @@ type LogOptions struct { Level int } -// Do sends request to this service. Any required authentication happens automatically. +// Do sends request to this service. Authentication of the request happens automatically. func (s *Service) Do(request *http.Request, timeout time.Duration) (*http.Response, error) { - if s.TLSOptions.AthenzDomain != "" && s.TLSOptions.KeyPair != nil { - accessToken, err := s.zts.AccessToken(s.TLSOptions.AthenzDomain, s.TLSOptions.KeyPair[0]) - if err != nil { + s.once.Do(func() { + util.ConfigureTLS(s.httpClient, s.TLSOptions.KeyPair, s.TLSOptions.CACertificate, s.TLSOptions.TrustAll) + }) + if s.auth != nil { + if err := s.auth.Authenticate(request); err != nil { return nil, err } - if request.Header == nil { - request.Header = make(http.Header) - } - request.Header.Add("Authorization", "Bearer "+accessToken) } return s.httpClient.Do(request, timeout) } @@ -118,7 +123,7 @@ func (s *Service) Wait(timeout time.Duration) (int, error) { default: return 0, fmt.Errorf("invalid service: %s", s.Name) } - return waitForOK(s.httpClient, url, s.TLSOptions.KeyPair, timeout) + return waitForOK(s, url, timeout) } func (s *Service) Description() string { @@ -133,27 +138,40 @@ func (s *Service) Description() string { return fmt.Sprintf("No description of service %s", s.Name) } -func isOK(status int) bool { return status/100 == 2 } +func isOK(status int) (bool, error) { + class := status / 100 + switch class { + case 2: // success + return true, nil + case 4: // client error + return false, fmt.Errorf("request failed with status %d", status) + default: // retry + return false, nil + } +} type responseFunc func(status int, response []byte) (bool, error) type requestFunc func() *http.Request -// waitForOK queries url and returns its status code. If the url returns a non-200 status code, it is repeatedly queried +// waitForOK queries url and returns its status code. If response status is not 2xx or 4xx, it is repeatedly queried // until timeout elapses. -func waitForOK(client util.HTTPClient, url string, certificates []tls.Certificate, timeout time.Duration) (int, error) { +func waitForOK(service *Service, url string, timeout time.Duration) (int, error) { req, err := http.NewRequest("GET", url, nil) if err != nil { return 0, err } - okFunc := func(status int, response []byte) (bool, error) { return isOK(status), nil } - return wait(client, okFunc, func() *http.Request { return req }, certificates, timeout) + okFunc := func(status int, response []byte) (bool, error) { + ok, err := isOK(status) + if err != nil { + return false, fmt.Errorf("failed to query %s at %s: %w", service.Description(), url, err) + } + return ok, err + } + return wait(service, okFunc, func() *http.Request { return req }, timeout) } -func wait(client util.HTTPClient, fn responseFunc, reqFn requestFunc, certificates []tls.Certificate, timeout time.Duration) (int, error) { - if certificates != nil { - util.SetCertificates(client, certificates) - } +func wait(service *Service, fn responseFunc, reqFn requestFunc, timeout time.Duration) (int, error) { var ( httpErr error response *http.Response @@ -163,7 +181,7 @@ func wait(client util.HTTPClient, fn responseFunc, reqFn requestFunc, certificat loopOnce := timeout == 0 for time.Now().Before(deadline) || loopOnce { req := reqFn() - response, httpErr = client.Do(req, 10*time.Second) + response, httpErr = service.Do(req, 10*time.Second) if httpErr == nil { statusCode = response.StatusCode body, err := io.ReadAll(response.Body) diff --git a/client/go/internal/vespa/target_cloud.go b/client/go/internal/vespa/target_cloud.go index 1fb3edd78c5..928bb788494 100644 --- a/client/go/internal/vespa/target_cloud.go +++ b/client/go/internal/vespa/target_cloud.go @@ -2,7 +2,6 @@ package vespa import ( "bytes" - "crypto/tls" "encoding/json" "fmt" "math" @@ -35,8 +34,8 @@ type cloudTarget struct { deploymentOptions CloudDeploymentOptions logOptions LogOptions httpClient util.HTTPClient - zts zts - auth0 auth0 + apiAuth Authenticator + deploymentAuth Authenticator } type deploymentEndpoint struct { @@ -62,23 +61,15 @@ type logMessage struct { Message string `json:"message"` } -type zts interface { - AccessToken(domain string, certficiate tls.Certificate) (string, error) -} - -type auth0 interface { - AccessToken() (string, error) -} - // CloudTarget creates a Target for the Vespa Cloud or hosted Vespa platform. -func CloudTarget(httpClient util.HTTPClient, ztsClient zts, auth0Client auth0, apiOptions APIOptions, deploymentOptions CloudDeploymentOptions, logOptions LogOptions) (Target, error) { +func CloudTarget(httpClient util.HTTPClient, apiAuth Authenticator, deploymentAuth Authenticator, apiOptions APIOptions, deploymentOptions CloudDeploymentOptions, logOptions LogOptions) (Target, error) { return &cloudTarget{ httpClient: httpClient, apiOptions: apiOptions, deploymentOptions: deploymentOptions, logOptions: logOptions, - zts: ztsClient, - auth0: auth0Client, + apiAuth: apiAuth, + deploymentAuth: deploymentAuth, }, nil } @@ -118,25 +109,25 @@ func (t *cloudTarget) IsCloud() bool { return true } func (t *cloudTarget) Deployment() Deployment { return t.deploymentOptions.Deployment } func (t *cloudTarget) Service(name string, timeout time.Duration, runID int64, cluster string) (*Service, error) { - var service *Service switch name { case DeployService: - service = &Service{ + service := &Service{ Name: name, BaseURL: t.apiOptions.System.URL, TLSOptions: t.apiOptions.TLSOptions, - zts: t.zts, httpClient: t.httpClient, + auth: t.apiAuth, } if timeout > 0 { status, err := service.Wait(timeout) if err != nil { return nil, err } - if !isOK(status) { + if ok, _ := isOK(status); !ok { return nil, fmt.Errorf("got status %d from deploy service at %s", status, service.BaseURL) } } + return service, nil case QueryService, DocumentService: if t.deploymentOptions.ClusterURLs == nil { if err := t.waitForEndpoints(timeout, runID); err != nil { @@ -147,38 +138,15 @@ func (t *cloudTarget) Service(name string, timeout time.Duration, runID int64, c if err != nil { return nil, err } - t.deploymentOptions.TLSOptions.AthenzDomain = t.apiOptions.System.AthenzDomain - service = &Service{ + return &Service{ Name: name, BaseURL: url, TLSOptions: t.deploymentOptions.TLSOptions, - zts: t.zts, httpClient: t.httpClient, - } - + auth: t.deploymentAuth, + }, nil default: return nil, fmt.Errorf("unknown service: %s", name) - - } - if service.TLSOptions.KeyPair != nil { - util.SetCertificates(service.httpClient, service.TLSOptions.KeyPair) - } - return service, nil -} - -func (t *cloudTarget) SignRequest(req *http.Request, keyID string) error { - if t.apiOptions.System.IsPublic() { - if t.apiOptions.APIKey != nil { - signer := NewRequestSigner(keyID, t.apiOptions.APIKey) - return signer.SignRequest(req) - } else { - return t.addAuth0AccessToken(req) - } - } else { - if t.apiOptions.TLSOptions.KeyPair == nil { - return fmt.Errorf("system %s requires a certificate for authentication", t.apiOptions.System.Name) - } - return nil } } @@ -190,7 +158,11 @@ func (t *cloudTarget) CheckVersion(clientVersion version.Version) error { if err != nil { return err } - response, err := t.httpClient.Do(req, 10*time.Second) + deployService, err := t.Service(DeployService, 0, 0, "") + if err != nil { + return err + } + response, err := deployService.Do(req, 10*time.Second) if err != nil { return err } @@ -212,18 +184,6 @@ func (t *cloudTarget) CheckVersion(clientVersion version.Version) error { return nil } -func (t *cloudTarget) addAuth0AccessToken(request *http.Request) error { - accessToken, err := t.auth0.AccessToken() - if err != nil { - return err - } - if request.Header == nil { - request.Header = make(http.Header) - } - request.Header.Set("Authorization", "Bearer "+accessToken) - return nil -} - func (t *cloudTarget) logsURL() string { return fmt.Sprintf("%s/application/v4/tenant/%s/application/%s/instance/%s/environment/%s/region/%s/logs", t.apiOptions.System.URL, @@ -246,11 +206,10 @@ func (t *cloudTarget) PrintLog(options LogOptions) error { q.Set("to", strconv.FormatInt(toMillis, 10)) } req.URL.RawQuery = q.Encode() - t.SignRequest(req, t.deploymentOptions.Deployment.Application.SerializedForm()) return req } logFunc := func(status int, response []byte) (bool, error) { - if ok, err := isCloudOK(status); !ok { + if ok, err := isOK(status); !ok { return ok, err } logEntries, err := ReadLogEntries(bytes.NewReader(response)) @@ -275,10 +234,18 @@ func (t *cloudTarget) PrintLog(options LogOptions) error { if options.Follow { timeout = math.MaxInt64 // No timeout } - _, err = wait(t.httpClient, logFunc, requestFunc, t.apiOptions.TLSOptions.KeyPair, timeout) + _, err = t.deployServiceWait(logFunc, requestFunc, timeout) return err } +func (t *cloudTarget) deployServiceWait(fn responseFunc, reqFn requestFunc, timeout time.Duration) (int, error) { + deployService, err := t.Service(DeployService, 0, 0, "") + if err != nil { + return 0, err + } + return wait(deployService, fn, reqFn, timeout) +} + func (t *cloudTarget) waitForEndpoints(timeout time.Duration, runID int64) error { if runID > 0 { if err := t.waitForRun(runID, timeout); err != nil { @@ -302,13 +269,10 @@ func (t *cloudTarget) waitForRun(runID int64, timeout time.Duration) error { q := req.URL.Query() q.Set("after", strconv.FormatInt(lastID, 10)) req.URL.RawQuery = q.Encode() - if err := t.SignRequest(req, t.deploymentOptions.Deployment.Application.SerializedForm()); err != nil { - util.JustExitWith(err) - } return req } jobSuccessFunc := func(status int, response []byte) (bool, error) { - if ok, err := isCloudOK(status); !ok { + if ok, err := isOK(status); !ok { return ok, err } var resp jobResponse @@ -326,7 +290,7 @@ func (t *cloudTarget) waitForRun(runID int64, timeout time.Duration) error { } return true, nil } - _, err = wait(t.httpClient, jobSuccessFunc, requestFunc, t.apiOptions.TLSOptions.KeyPair, timeout) + _, err = t.deployServiceWait(jobSuccessFunc, requestFunc, timeout) return err } @@ -361,12 +325,9 @@ func (t *cloudTarget) discoverEndpoints(timeout time.Duration) error { if err != nil { return err } - if err := t.SignRequest(req, t.deploymentOptions.Deployment.Application.SerializedForm()); err != nil { - return err - } urlsByCluster := make(map[string]string) endpointFunc := func(status int, response []byte) (bool, error) { - if ok, err := isCloudOK(status); !ok { + if ok, err := isOK(status); !ok { return ok, err } var resp deploymentResponse @@ -384,7 +345,7 @@ func (t *cloudTarget) discoverEndpoints(timeout time.Duration) error { } return true, nil } - if _, err = wait(t.httpClient, endpointFunc, func() *http.Request { return req }, t.apiOptions.TLSOptions.KeyPair, timeout); err != nil { + if _, err := t.deployServiceWait(endpointFunc, func() *http.Request { return req }, timeout); err != nil { return err } if len(urlsByCluster) == 0 { @@ -393,11 +354,3 @@ func (t *cloudTarget) discoverEndpoints(timeout time.Duration) error { t.deploymentOptions.ClusterURLs = urlsByCluster return nil } - -func isCloudOK(status int) (bool, error) { - if status == 401 { - // when retrying we should give up immediately if we're not authorized - return false, fmt.Errorf("status %d: invalid credentials", status) - } - return isOK(status), nil -} diff --git a/client/go/internal/vespa/target_custom.go b/client/go/internal/vespa/target_custom.go index 848d19f0a90..0a3a9d48fed 100644 --- a/client/go/internal/vespa/target_custom.go +++ b/client/go/internal/vespa/target_custom.go @@ -15,6 +15,7 @@ type customTarget struct { targetType string baseURL string httpClient util.HTTPClient + tlsOptions TLSOptions } type serviceConvergeResponse struct { @@ -22,13 +23,13 @@ type serviceConvergeResponse struct { } // LocalTarget creates a target for a Vespa platform running locally. -func LocalTarget(httpClient util.HTTPClient) Target { - return &customTarget{targetType: TargetLocal, baseURL: "http://127.0.0.1", httpClient: httpClient} +func LocalTarget(httpClient util.HTTPClient, tlsOptions TLSOptions) Target { + return &customTarget{targetType: TargetLocal, baseURL: "http://127.0.0.1", httpClient: httpClient, tlsOptions: tlsOptions} } // CustomTarget creates a Target for a Vespa platform running at baseURL. -func CustomTarget(httpClient util.HTTPClient, baseURL string) Target { - return &customTarget{targetType: TargetCustom, baseURL: baseURL, httpClient: httpClient} +func CustomTarget(httpClient util.HTTPClient, baseURL string, tlsOptions TLSOptions) Target { + return &customTarget{targetType: TargetCustom, baseURL: baseURL, httpClient: httpClient, tlsOptions: tlsOptions} } func (t *customTarget) Type() string { return t.targetType } @@ -44,7 +45,7 @@ func (t *customTarget) createService(name string) (*Service, error) { if err != nil { return nil, err } - return &Service{BaseURL: url, Name: name, httpClient: t.httpClient}, nil + return &Service{BaseURL: url, Name: name, httpClient: t.httpClient, TLSOptions: t.tlsOptions}, nil } return nil, fmt.Errorf("unknown service: %s", name) } @@ -60,7 +61,7 @@ func (t *customTarget) Service(name string, timeout time.Duration, sessionOrRunI if err != nil { return nil, err } - if !isOK(status) { + if ok, _ := isOK(status); !ok { return nil, fmt.Errorf("got status %d from deploy service at %s", status, service.BaseURL) } } else { @@ -76,8 +77,6 @@ func (t *customTarget) PrintLog(options LogOptions) error { return fmt.Errorf("log access is only supported on cloud: run vespa-logfmt on the admin node instead") } -func (t *customTarget) SignRequest(req *http.Request, sigKeyId string) error { return nil } - func (t *customTarget) CheckVersion(version version.Version) error { return nil } func (t *customTarget) urlWithPort(serviceName string) (string, error) { @@ -101,19 +100,19 @@ func (t *customTarget) urlWithPort(serviceName string) (string, error) { } func (t *customTarget) waitForConvergence(timeout time.Duration) error { - deployURL, err := t.urlWithPort(DeployService) + deployService, err := t.createService(DeployService) if err != nil { return err } - url := fmt.Sprintf("%s/application/v2/tenant/default/application/default/environment/prod/region/default/instance/default/serviceconverge", deployURL) + url := fmt.Sprintf("%s/application/v2/tenant/default/application/default/environment/prod/region/default/instance/default/serviceconverge", deployService.BaseURL) req, err := http.NewRequest("GET", url, nil) if err != nil { return err } converged := false convergedFunc := func(status int, response []byte) (bool, error) { - if !isOK(status) { - return false, nil + if ok, err := isOK(status); !ok { + return ok, err } var resp serviceConvergeResponse if err := json.Unmarshal(response, &resp); err != nil { @@ -122,7 +121,7 @@ func (t *customTarget) waitForConvergence(timeout time.Duration) error { converged = resp.Converged return converged, nil } - if _, err := wait(t.httpClient, convergedFunc, func() *http.Request { return req }, nil, timeout); err != nil { + if _, err := wait(deployService, convergedFunc, func() *http.Request { return req }, timeout); err != nil { return err } if !converged { diff --git a/client/go/internal/vespa/target_test.go b/client/go/internal/vespa/target_test.go index b9d65f3d8a4..bf266e8f9ec 100644 --- a/client/go/internal/vespa/target_test.go +++ b/client/go/internal/vespa/target_test.go @@ -3,7 +3,6 @@ package vespa import ( "bytes" - "crypto/tls" "fmt" "io" "net/http" @@ -19,10 +18,16 @@ import ( type mockVespaApi struct { deploymentConverged bool + authFailure bool serverURL string } func (v *mockVespaApi) mockVespaHandler(w http.ResponseWriter, req *http.Request) { + if v.authFailure { + response := `{"message":"unauthorized"}` + w.WriteHeader(401) + w.Write([]byte(response)) + } switch req.URL.Path { case "/cli/v1/": response := `{"minVersion":"8.0.0"}` @@ -65,17 +70,17 @@ func (v *mockVespaApi) mockVespaHandler(w http.ResponseWriter, req *http.Request } func TestCustomTarget(t *testing.T) { - lt := LocalTarget(&mock.HTTPClient{}) + lt := LocalTarget(&mock.HTTPClient{}, TLSOptions{}) assertServiceURL(t, "http://127.0.0.1:19071", lt, "deploy") assertServiceURL(t, "http://127.0.0.1:8080", lt, "query") assertServiceURL(t, "http://127.0.0.1:8080", lt, "document") - ct := CustomTarget(&mock.HTTPClient{}, "http://192.0.2.42") + ct := CustomTarget(&mock.HTTPClient{}, "http://192.0.2.42", TLSOptions{}) assertServiceURL(t, "http://192.0.2.42:19071", ct, "deploy") assertServiceURL(t, "http://192.0.2.42:8080", ct, "query") assertServiceURL(t, "http://192.0.2.42:8080", ct, "document") - ct2 := CustomTarget(&mock.HTTPClient{}, "http://192.0.2.42:60000") + ct2 := CustomTarget(&mock.HTTPClient{}, "http://192.0.2.42:60000", TLSOptions{}) assertServiceURL(t, "http://192.0.2.42:60000", ct2, "deploy") assertServiceURL(t, "http://192.0.2.42:60000", ct2, "query") assertServiceURL(t, "http://192.0.2.42:60000", ct2, "document") @@ -85,7 +90,7 @@ func TestCustomTargetWait(t *testing.T) { vc := mockVespaApi{} srv := httptest.NewServer(http.HandlerFunc(vc.mockVespaHandler)) defer srv.Close() - target := CustomTarget(util.CreateClient(time.Second*10), srv.URL) + target := CustomTarget(util.CreateClient(time.Second*10), srv.URL, TLSOptions{}) _, err := target.Service("query", time.Millisecond, 42, "") assert.NotNil(t, err) @@ -107,6 +112,9 @@ func TestCloudTargetWait(t *testing.T) { var logWriter bytes.Buffer target := createCloudTarget(t, srv.URL, &logWriter) + vc.authFailure = true + assertServiceWaitErr(t, 401, true, target, "deploy") + vc.authFailure = false assertServiceWait(t, 200, target, "deploy") _, err := target.Service("query", time.Millisecond, 42, "") @@ -157,10 +165,11 @@ func createCloudTarget(t *testing.T, url string, logWriter io.Writer) Target { apiKey, err := CreateAPIKey() assert.Nil(t, err) + auth := &mockAuthenticator{} target, err := CloudTarget( util.CreateClient(time.Second*10), - &mockZTS{}, - &mockAuth0{}, + auth, + auth, APIOptions{APIKey: apiKey, System: PublicSystem}, CloudDeploymentOptions{ Deployment: Deployment{ @@ -175,7 +184,6 @@ func createCloudTarget(t *testing.T, url string, logWriter io.Writer) Target { } if ct, ok := target.(*cloudTarget); ok { ct.apiOptions.System.URL = url - ct.zts = &mockZTS{token: "foo bar"} } else { t.Fatalf("Wrong target type %T", ct) } @@ -189,22 +197,22 @@ func assertServiceURL(t *testing.T, url string, target Target, service string) { } func assertServiceWait(t *testing.T, expectedStatus int, target Target, service string) { + assertServiceWaitErr(t, expectedStatus, false, target, service) +} + +func assertServiceWaitErr(t *testing.T, expectedStatus int, expectErr bool, target Target, service string) { s, err := target.Service(service, 0, 42, "") assert.Nil(t, err) status, err := s.Wait(0) - assert.Nil(t, err) + if expectErr { + assert.NotNil(t, err) + } else { + assert.Nil(t, err) + } assert.Equal(t, expectedStatus, status) } -type mockZTS struct{ token string } - -func (c *mockZTS) AccessToken(domain string, certificate tls.Certificate) (string, error) { - return c.token, nil -} - -type mockAuth0 struct{} - -func (a *mockAuth0) AccessToken() (string, error) { return "", nil } +type mockAuthenticator struct{} -func (a *mockAuth0) HasCredentials() bool { return true } +func (a *mockAuthenticator) Authenticate(request *http.Request) error { return nil } diff --git a/cloud-tenant-base-dependencies-enforcer/pom.xml b/cloud-tenant-base-dependencies-enforcer/pom.xml index daf3683ad26..4d5d801e0e3 100644 --- a/cloud-tenant-base-dependencies-enforcer/pom.xml +++ b/cloud-tenant-base-dependencies-enforcer/pom.xml @@ -46,7 +46,7 @@ <jaxb.version>2.3.0</jaxb.version> <jetty.version>11.0.14</jetty.version> <org.lz4.version>1.8.0</org.lz4.version> - <org.json.version>20220320</org.json.version> <!-- TODO: Remove on Vespa 9 --> + <org.json.version>20230227</org.json.version> <!-- TODO: Remove on Vespa 9 --> <slf4j.version>1.7.32</slf4j.version> <!-- WARNING: when updated, also update c.y.v.tenant:base pom --> <xml-apis.version>1.4.01</xml-apis.version> </properties> diff --git a/config-model-api/src/main/java/com/yahoo/config/model/api/ModelContext.java b/config-model-api/src/main/java/com/yahoo/config/model/api/ModelContext.java index fef2354c452..7f2dd4b6acd 100644 --- a/config-model-api/src/main/java/com/yahoo/config/model/api/ModelContext.java +++ b/config-model-api/src/main/java/com/yahoo/config/model/api/ModelContext.java @@ -113,6 +113,7 @@ public interface ModelContext { @ModelFeatureFlag(owners = {"tokle, bjorncs"}, removeAfter = "8.108") default boolean enableDataPlaneFilter() { return true; } @ModelFeatureFlag(owners = {"arnej, bjorncs"}) default boolean enableGlobalPhase() { return true; } @ModelFeatureFlag(owners = {"baldersheim"}, comment = "Select summary decode type") default String summaryDecodePolicy() { return "eager"; } + @ModelFeatureFlag(owners = {"hmusum"}) default boolean allowMoreThanOneContentGroupDown(ClusterSpec.Id id) { return false; } //Below are all flags that must be kept until 7 is out of the door @ModelFeatureFlag(owners = {"arnej"}, removeAfter="7.last") default boolean ignoreThreadStackSizes() { return false; } diff --git a/config-model/src/main/java/com/yahoo/schema/derived/VsmFields.java b/config-model/src/main/java/com/yahoo/schema/derived/VsmFields.java index c8679b6166c..c032a7155b2 100644 --- a/config-model/src/main/java/com/yahoo/schema/derived/VsmFields.java +++ b/config-model/src/main/java/com/yahoo/schema/derived/VsmFields.java @@ -13,12 +13,14 @@ import com.yahoo.document.datatypes.StringFieldValue; import com.yahoo.document.datatypes.TensorFieldValue; import com.yahoo.schema.FieldSets; import com.yahoo.schema.Schema; +import com.yahoo.schema.document.Attribute; import com.yahoo.schema.document.FieldSet; import com.yahoo.schema.document.GeoPos; import com.yahoo.schema.document.Matching; import com.yahoo.schema.document.MatchType; import com.yahoo.schema.document.SDDocumentType; import com.yahoo.schema.document.SDField; +import com.yahoo.schema.processing.TensorFieldProcessor; import com.yahoo.vespa.config.search.vsm.VsmfieldsConfig; import java.util.LinkedHashMap; @@ -124,63 +126,68 @@ public class VsmFields extends Derived implements VsmfieldsConfig.Producer { private final Type type; private final boolean isAttribute; + private final Attribute.DistanceMetric distanceMetric; /** The streaming field type enumeration */ public static class Type { - public static Type INT8 = new Type("int8","INT8"); - public static Type INT16 = new Type("int16","INT16"); - public static Type INT32 = new Type("int32","INT32"); - public static Type INT64 = new Type("int64","INT64"); - public static Type FLOAT16 = new Type("float16", "FLOAT16"); - public static Type FLOAT = new Type("float","FLOAT"); - public static Type DOUBLE = new Type("double","DOUBLE"); - public static Type STRING = new Type("string","AUTOUTF8"); - public static Type BOOL = new Type("bool","BOOL"); - public static Type UNSEARCHABLESTRING = new Type("string","NONE"); - public static Type GEO_POSITION = new Type("position", "GEOPOS"); - - private String name; + public static Type INT8 = new Type("INT8"); + public static Type INT16 = new Type("INT16"); + public static Type INT32 = new Type("INT32"); + public static Type INT64 = new Type("INT64"); + public static Type FLOAT16 = new Type("FLOAT16"); + public static Type FLOAT = new Type("FLOAT"); + public static Type DOUBLE = new Type("DOUBLE"); + public static Type STRING = new Type("AUTOUTF8"); + public static Type BOOL = new Type("BOOL"); + public static Type UNSEARCHABLESTRING = new Type("NONE"); + public static Type GEO_POSITION = new Type("GEOPOS"); + public static Type NEAREST_NEIGHBOR = new Type("NEAREST_NEIGHBOR"); private String searchMethod; - private Type(String name, String searchMethod) { - this.name = name; + private Type(String searchMethod) { this.searchMethod = searchMethod; } @Override public int hashCode() { - return name.hashCode(); + return searchMethod.hashCode(); } - /** Returns the name of this type */ - public String getName() { return name; } - public String getSearchMethod() { return searchMethod; } @Override public boolean equals(Object other) { if ( ! (other instanceof Type)) return false; - return this.name.equals(((Type)other).name); + return this.searchMethod.equals(((Type)other).searchMethod); } @Override public String toString() { - return "type: " + name; + return "method: " + searchMethod; } } public StreamingField(SDField field) { - this(field.getName(), field.getDataType(), field.getMatching(), field.doesAttributing()); + this(field.getName(), field.getDataType(), field.getMatching(), field.doesAttributing(), getDistanceMetric(field)); } - private StreamingField(String name, DataType sourceType, Matching matching, boolean isAttribute) { + private StreamingField(String name, DataType sourceType, Matching matching, boolean isAttribute, Attribute.DistanceMetric distanceMetric) { this.name = name; this.type = convertType(sourceType); this.matching = matching; this.isAttribute = isAttribute; + this.distanceMetric = distanceMetric; + } + + private static Attribute.DistanceMetric getDistanceMetric(SDField field) { + var attr = field.getAttribute(); + if (attr != null) { + return attr.distanceMetric(); + } + return Attribute.DEFAULT_DISTANCE_METRIC; } /** Converts to the right index type from a field datatype */ @@ -211,6 +218,10 @@ public class VsmFields extends Derived implements VsmfieldsConfig.Producer { } else if (fval instanceof PredicateFieldValue) { return Type.UNSEARCHABLESTRING; } else if (fval instanceof TensorFieldValue) { + var tensorType = ((TensorFieldValue) fval).getDataType().getTensorType(); + if (TensorFieldProcessor.isTensorTypeThatSupportsHnswIndex(tensorType)) { + return Type.NEAREST_NEIGHBOR; + } return Type.UNSEARCHABLESTRING; } else if (fieldType instanceof CollectionDataType) { return convertType(((CollectionDataType) fieldType).getNestedType()); @@ -224,8 +235,7 @@ public class VsmFields extends Derived implements VsmfieldsConfig.Producer { public String getName() { return name; } - public VsmfieldsConfig.Fieldspec.Builder getFieldSpecConfig() { - VsmfieldsConfig.Fieldspec.Builder fB = new VsmfieldsConfig.Fieldspec.Builder(); + public String getMatchingName() { String matchingName = matching.getType().getName(); if (matching.getType().equals(MatchType.TEXT)) matchingName = ""; @@ -241,9 +251,21 @@ public class VsmFields extends Derived implements VsmfieldsConfig.Producer { if (type != Type.STRING) { matchingName = ""; } + return matchingName; + } + + public String getArg1() { + if (type == Type.NEAREST_NEIGHBOR) { + return distanceMetric.name(); + } + return getMatchingName(); + } + + public VsmfieldsConfig.Fieldspec.Builder getFieldSpecConfig() { + var fB = new VsmfieldsConfig.Fieldspec.Builder(); fB.name(getName()) .searchmethod(VsmfieldsConfig.Fieldspec.Searchmethod.Enum.valueOf(type.getSearchMethod())) - .arg1(matchingName) + .arg1(getArg1()) .fieldtype(isAttribute ? VsmfieldsConfig.Fieldspec.Fieldtype.ATTRIBUTE : VsmfieldsConfig.Fieldspec.Fieldtype.INDEX); diff --git a/config-model/src/main/java/com/yahoo/schema/expressiontransforms/InputRecorder.java b/config-model/src/main/java/com/yahoo/schema/expressiontransforms/InputRecorder.java index 7f578f07fe3..5d3624cd3d3 100644 --- a/config-model/src/main/java/com/yahoo/schema/expressiontransforms/InputRecorder.java +++ b/config-model/src/main/java/com/yahoo/schema/expressiontransforms/InputRecorder.java @@ -120,11 +120,7 @@ public class InputRecorder extends ExpressionTransformer<InputRecorderContext> { if (model == null) { throw new IllegalArgumentException("missing onnx model: " + arg); } - model.getInputMap().forEach((onnxName, onnxInput) -> { - if (model.getInitializers().contains(onnxName)) { - log.fine(() -> "For input '%s': skipping name '%s' as it's an initializer".formatted(onnxInput, onnxName)); - return; - } + model.getInputMap().forEach((__, onnxInput) -> { var reader = new StringReader(onnxInput); try { var asExpression = new RankingExpression(reader); diff --git a/config-model/src/main/java/com/yahoo/schema/processing/TensorFieldProcessor.java b/config-model/src/main/java/com/yahoo/schema/processing/TensorFieldProcessor.java index 37da07f8227..227054d9800 100644 --- a/config-model/src/main/java/com/yahoo/schema/processing/TensorFieldProcessor.java +++ b/config-model/src/main/java/com/yahoo/schema/processing/TensorFieldProcessor.java @@ -9,6 +9,7 @@ import com.yahoo.schema.Schema; import com.yahoo.schema.document.HnswIndexParams; import com.yahoo.schema.document.ImmutableSDField; import com.yahoo.schema.document.SDField; +import com.yahoo.tensor.TensorType; import com.yahoo.vespa.model.container.search.QueryProfiles; /** @@ -50,6 +51,14 @@ public class TensorFieldProcessor extends Processor { private boolean isTensorTypeThatSupportsHnswIndex(ImmutableSDField field) { var type = ((TensorDataType)field.getDataType()).getTensorType(); + return isTensorTypeThatSupportsHnswIndex(type); + } + + /** + * Returns whether the given tensor type supports using HNSW index and + * nearest neighbor search. + */ + public static boolean isTensorTypeThatSupportsHnswIndex(TensorType type) { // Tensors with 1 indexed dimension support hnsw index (used for approximate nearest neighbor search). if ((type.dimensions().size() == 1) && type.dimensions().get(0).isIndexed()) { diff --git a/config-model/src/main/java/com/yahoo/vespa/model/application/validation/StreamingValidator.java b/config-model/src/main/java/com/yahoo/vespa/model/application/validation/StreamingValidator.java index 773d696f3e8..ad126cfa22b 100644 --- a/config-model/src/main/java/com/yahoo/vespa/model/application/validation/StreamingValidator.java +++ b/config-model/src/main/java/com/yahoo/vespa/model/application/validation/StreamingValidator.java @@ -5,6 +5,7 @@ import com.yahoo.config.application.api.DeployLogger; import com.yahoo.config.model.deploy.DeployState; import com.yahoo.document.DataType; import com.yahoo.document.NumericDataType; +import com.yahoo.document.TensorDataType; import com.yahoo.documentmodel.NewDocumentReferenceDataType; import com.yahoo.schema.document.Attribute; import com.yahoo.schema.document.ImmutableSDField; @@ -63,6 +64,8 @@ public class StreamingValidator extends Validator { // If the field is numeric, we can't print this, because we may have converted the field to // attribute indexing ourselves (IntegerIndex2Attribute) if (sd.getDataType() instanceof NumericDataType) return; + // Tensor fields are only searchable via nearest neighbor search, and match semantics are irrelevant. + if (sd.getDataType() instanceof TensorDataType) return; logger.logApplicationPackage(Level.WARNING, "For streaming search cluster '" + sc.getClusterName() + "', SD field '" + sd.getName() + "': 'attribute' has same match semantics as 'index'."); diff --git a/config-model/src/main/java/com/yahoo/vespa/model/content/ClusterControllerConfig.java b/config-model/src/main/java/com/yahoo/vespa/model/content/ClusterControllerConfig.java index 8ec4ae35658..201e0b5693a 100644 --- a/config-model/src/main/java/com/yahoo/vespa/model/content/ClusterControllerConfig.java +++ b/config-model/src/main/java/com/yahoo/vespa/model/content/ClusterControllerConfig.java @@ -14,8 +14,6 @@ import org.w3c.dom.Element; /** * Config generation for common parameters for all fleet controllers. - * - * TODO: Author */ public class ClusterControllerConfig extends AnyConfigProducer implements FleetcontrollerConfig.Producer { @@ -23,11 +21,16 @@ public class ClusterControllerConfig extends AnyConfigProducer implements Fleetc private final String clusterName; private final ModelElement clusterElement; private final ResourceLimits resourceLimits; + private final boolean allowMoreThanOneContentGroupDown; - public Builder(String clusterName, ModelElement clusterElement, ResourceLimits resourceLimits) { + public Builder(String clusterName, + ModelElement clusterElement, + ResourceLimits resourceLimits, + boolean allowMoreThanOneContentGroupDown) { this.clusterName = clusterName; this.clusterElement = clusterElement; this.resourceLimits = resourceLimits; + this.allowMoreThanOneContentGroupDown = allowMoreThanOneContentGroupDown; } @Override @@ -53,13 +56,15 @@ public class ClusterControllerConfig extends AnyConfigProducer implements Fleetc tuning.childAsDouble("min-storage-up-ratio"), bucketSplittingMinimumBits, minNodeRatioPerGroup, - resourceLimits); + resourceLimits, + allowMoreThanOneContentGroupDown); } else { return new ClusterControllerConfig(ancestor, clusterName, null, null, null, null, null, null, bucketSplittingMinimumBits, minNodeRatioPerGroup, - resourceLimits); + resourceLimits, + allowMoreThanOneContentGroupDown); } } } @@ -74,6 +79,7 @@ public class ClusterControllerConfig extends AnyConfigProducer implements Fleetc private final Integer minSplitBits; private final Double minNodeRatioPerGroup; private final ResourceLimits resourceLimits; + private final boolean allowMoreThanOneContentGroupDown; // TODO refactor; too many args private ClusterControllerConfig(TreeConfigProducer<?> parent, @@ -86,7 +92,8 @@ public class ClusterControllerConfig extends AnyConfigProducer implements Fleetc Double minStorageUpRatio, Integer minSplitBits, Double minNodeRatioPerGroup, - ResourceLimits resourceLimits) { + ResourceLimits resourceLimits, + boolean allowMoreThanOneContentGroupDown) { super(parent, "fleetcontroller"); this.clusterName = clusterName; @@ -99,6 +106,7 @@ public class ClusterControllerConfig extends AnyConfigProducer implements Fleetc this.minSplitBits = minSplitBits; this.minNodeRatioPerGroup = minNodeRatioPerGroup; this.resourceLimits = resourceLimits; + this.allowMoreThanOneContentGroupDown = allowMoreThanOneContentGroupDown; } @Override @@ -141,6 +149,7 @@ public class ClusterControllerConfig extends AnyConfigProducer implements Fleetc builder.min_node_ratio_per_group(minNodeRatioPerGroup); } resourceLimits.getConfig(builder); + builder.max_number_of_groups_allowed_to_be_down(allowMoreThanOneContentGroupDown ? 1 : -1); } } diff --git a/config-model/src/main/java/com/yahoo/vespa/model/content/cluster/ContentCluster.java b/config-model/src/main/java/com/yahoo/vespa/model/content/cluster/ContentCluster.java index 7f4fc4cd89d..217c26516a9 100644 --- a/config-model/src/main/java/com/yahoo/vespa/model/content/cluster/ContentCluster.java +++ b/config-model/src/main/java/com/yahoo/vespa/model/content/cluster/ContentCluster.java @@ -127,7 +127,8 @@ public class ContentCluster extends TreeConfigProducer<AnyConfigProducer> implem .build(contentElement); c.clusterControllerConfig = new ClusterControllerConfig.Builder(clusterId, contentElement, - resourceLimits.getClusterControllerLimits()) + resourceLimits.getClusterControllerLimits(), + deployState.featureFlags().allowMoreThanOneContentGroupDown(new ClusterSpec.Id(clusterId))) .build(deployState, c, contentElement.getXml()); c.search = new ContentSearchCluster.Builder(documentDefinitions, globallyDistributedDocuments, diff --git a/config-model/src/main/java/com/yahoo/vespa/model/ml/OnnxModelInfo.java b/config-model/src/main/java/com/yahoo/vespa/model/ml/OnnxModelInfo.java index 7c89a349d7d..1984ceadac6 100644 --- a/config-model/src/main/java/com/yahoo/vespa/model/ml/OnnxModelInfo.java +++ b/config-model/src/main/java/com/yahoo/vespa/model/ml/OnnxModelInfo.java @@ -23,6 +23,7 @@ import java.util.HashSet; import java.util.List; import java.util.Map; import java.util.Set; +import java.util.logging.Logger; import java.util.stream.Collectors; /** @@ -36,6 +37,8 @@ import java.util.stream.Collectors; */ public class OnnxModelInfo { + private static final Logger log = Logger.getLogger(OnnxModelInfo.class.getName()); + private final ApplicationPackage app; private final String modelPath; private final String defaultOutput; @@ -196,15 +199,27 @@ public class OnnxModelInfo { } static private String onnxModelToJson(Onnx.ModelProto model, Path path) throws IOException { + var initializerNames = model.getGraph().getInitializerList().stream() + .map(Onnx.TensorProto::getName).collect(Collectors.toSet()); ByteArrayOutputStream out = new ByteArrayOutputStream(); JsonGenerator g = new JsonFactory().createGenerator(out, JsonEncoding.UTF8); g.writeStartObject(); g.writeStringField("path", path.toString()); g.writeArrayFieldStart("inputs"); + int skippedInput = 0; for (Onnx.ValueInfoProto valueInfo : model.getGraph().getInputList()) { + if (initializerNames.contains(valueInfo.getName())) { + log.fine(() -> "For '%s': skipping name '%s' as it's an initializer" + .formatted(path.getName(), valueInfo.getName())); + ++skippedInput; + continue; + } onnxTypeToJson(g, valueInfo); } + if (skippedInput > 0) + log.info("For '%s': skipped %d inputs that were also listed in initializers" + .formatted(path.getName(), skippedInput)); g.writeEndArray(); g.writeArrayFieldStart("outputs"); diff --git a/config-model/src/main/java/com/yahoo/vespa/model/search/NodeResourcesTuning.java b/config-model/src/main/java/com/yahoo/vespa/model/search/NodeResourcesTuning.java index ee18eceb719..2de06e2053a 100644 --- a/config-model/src/main/java/com/yahoo/vespa/model/search/NodeResourcesTuning.java +++ b/config-model/src/main/java/com/yahoo/vespa/model/search/NodeResourcesTuning.java @@ -18,7 +18,6 @@ public class NodeResourcesTuning implements ProtonConfig.Producer { private final static double SUMMARY_CACHE_SIZE_AS_FRACTION_OF_MEMORY = 0.04; private final static double MEMORY_GAIN_AS_FRACTION_OF_MEMORY = 0.08; private final static double MIN_MEMORY_PER_FLUSH_THREAD_GB = 16.0; - private final static double MAX_FLUSH_THREAD_RATIO = 1.0/8; private final static double TLS_SIZE_FRACTION = 0.02; final static long MB = 1024 * 1024; public final static long GB = MB * 1024; @@ -94,13 +93,12 @@ public class NodeResourcesTuning implements ProtonConfig.Producer { } private void tuneFlushConcurrentThreads(ProtonConfig.Flush.Builder builder) { + int max_concurrent = 2; // TODO bring slowly up towards 4 if (usableMemoryGb() < MIN_MEMORY_PER_FLUSH_THREAD_GB) { - builder.maxconcurrent(1); + max_concurrent = 1; } - double min_concurrent_mem = usableMemoryGb() / (2*MIN_MEMORY_PER_FLUSH_THREAD_GB); - double min_concurrent_cpu = resources.vcpu() * MAX_FLUSH_THREAD_RATIO; - builder.maxconcurrent(Math.min(builder.build().maxconcurrent(), - (int)Math.ceil(Math.max(min_concurrent_mem, min_concurrent_cpu)))); + double min_concurrent_mem = usableMemoryGb() / MIN_MEMORY_PER_FLUSH_THREAD_GB; + builder.maxconcurrent(Math.min(max_concurrent, (int)Math.ceil(min_concurrent_mem))); } private void tuneFlushStrategyTlsSize(ProtonConfig.Flush.Memory.Builder builder) { diff --git a/config-model/src/test/derived/nearestneighbor_streaming/test.sd b/config-model/src/test/derived/nearestneighbor_streaming/test.sd new file mode 100644 index 00000000000..4427fa08ab6 --- /dev/null +++ b/config-model/src/test/derived/nearestneighbor_streaming/test.sd @@ -0,0 +1,24 @@ +# Copyright Yahoo. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. +schema test { + document test { + field vec_a type tensor<float>(x[16]) { + indexing: attribute + } + field vec_b type tensor<float>(x[16]) { + indexing: attribute + attribute { + distance-metric: angular + } + } + field vec_c type tensor<float>(m{},x[16]) { + indexing: attribute + attribute { + distance-metric: innerproduct + } + } + # This tensor field can not be used with nearest neighbor search. + field vec_d type tensor<float>(x{}) { + indexing: attribute + } + } +} diff --git a/config-model/src/test/derived/nearestneighbor_streaming/vsmfields.cfg b/config-model/src/test/derived/nearestneighbor_streaming/vsmfields.cfg new file mode 100644 index 00000000000..f8b1cf62048 --- /dev/null +++ b/config-model/src/test/derived/nearestneighbor_streaming/vsmfields.cfg @@ -0,0 +1,31 @@ +documentverificationlevel 0 +searchall 1 +fieldspec[].name "vec_a" +fieldspec[].searchmethod NEAREST_NEIGHBOR +fieldspec[].arg1 "EUCLIDEAN" +fieldspec[].maxlength 1048576 +fieldspec[].fieldtype ATTRIBUTE +fieldspec[].name "vec_b" +fieldspec[].searchmethod NEAREST_NEIGHBOR +fieldspec[].arg1 "ANGULAR" +fieldspec[].maxlength 1048576 +fieldspec[].fieldtype ATTRIBUTE +fieldspec[].name "vec_c" +fieldspec[].searchmethod NEAREST_NEIGHBOR +fieldspec[].arg1 "INNERPRODUCT" +fieldspec[].maxlength 1048576 +fieldspec[].fieldtype ATTRIBUTE +fieldspec[].name "vec_d" +fieldspec[].searchmethod NONE +fieldspec[].arg1 "" +fieldspec[].maxlength 1048576 +fieldspec[].fieldtype ATTRIBUTE +documenttype[].name "test" +documenttype[].index[].name "vec_a" +documenttype[].index[].field[].name "vec_a" +documenttype[].index[].name "vec_b" +documenttype[].index[].field[].name "vec_b" +documenttype[].index[].name "vec_c" +documenttype[].index[].field[].name "vec_c" +documenttype[].index[].name "vec_d" +documenttype[].index[].field[].name "vec_d" diff --git a/config-model/src/test/java/com/yahoo/schema/derived/NearestNeighborTestCase.java b/config-model/src/test/java/com/yahoo/schema/derived/NearestNeighborTestCase.java index b3a0b8d4558..713da6f5cbe 100644 --- a/config-model/src/test/java/com/yahoo/schema/derived/NearestNeighborTestCase.java +++ b/config-model/src/test/java/com/yahoo/schema/derived/NearestNeighborTestCase.java @@ -35,4 +35,9 @@ public class NearestNeighborTestCase extends AbstractExportingTestCase { } } + @Test + void test_nearest_neighbor_streaming() throws IOException, ParseException { + assertCorrectDeriving("nearestneighbor_streaming"); + } + } diff --git a/config-model/src/test/java/com/yahoo/vespa/model/content/FleetControllerClusterTest.java b/config-model/src/test/java/com/yahoo/vespa/model/content/FleetControllerClusterTest.java index 1e6847a47be..1f8dea41a3e 100644 --- a/config-model/src/test/java/com/yahoo/vespa/model/content/FleetControllerClusterTest.java +++ b/config-model/src/test/java/com/yahoo/vespa/model/content/FleetControllerClusterTest.java @@ -27,7 +27,8 @@ public class FleetControllerClusterTest { new ClusterResourceLimits.Builder(false, featureFlags.resourceLimitDisk(), featureFlags.resourceLimitMemory()) - .build(clusterElement).getClusterControllerLimits()) + .build(clusterElement).getClusterControllerLimits(), + false) .build(root.getDeployState(), root, clusterElement.getXml()); } diff --git a/config-model/src/test/java/com/yahoo/vespa/model/search/NodeResourcesTuningTest.java b/config-model/src/test/java/com/yahoo/vespa/model/search/NodeResourcesTuningTest.java index 9fe38512fc0..d344be3da9a 100644 --- a/config-model/src/test/java/com/yahoo/vespa/model/search/NodeResourcesTuningTest.java +++ b/config-model/src/test/java/com/yahoo/vespa/model/search/NodeResourcesTuningTest.java @@ -182,14 +182,12 @@ public class NodeResourcesTuningTest { } @Test public void require_that_concurrent_flush_threads_is_1_with_low_memory() { - assertEquals(2, fromMemAndCpu(17, 9).flush().maxconcurrent()); - assertEquals(2, fromMemAndCpu(17, 64).flush().maxconcurrent()); // still capped by max - assertEquals(2, fromMemAndCpu(65, 8).flush().maxconcurrent()); // still capped by max - assertEquals(2, fromMemAndCpu(33, 8).flush().maxconcurrent()); - assertEquals(1, fromMemAndCpu(31, 8).flush().maxconcurrent()); - assertEquals(1, fromMemAndCpu(15, 8).flush().maxconcurrent()); - assertEquals(1, fromMemAndCpu(17, 8).flush().maxconcurrent()); + assertEquals(1, fromMemAndCpu(1, 8).flush().maxconcurrent()); assertEquals(1, fromMemAndCpu(15, 8).flush().maxconcurrent()); + assertEquals(1, fromMemAndCpu(16, 8).flush().maxconcurrent()); + assertEquals(2, fromMemAndCpu(17, 8).flush().maxconcurrent()); + assertEquals(2, fromMemAndCpu(65, 8).flush().maxconcurrent()); // still capped by max + assertEquals(2, fromMemAndCpu(65, 65).flush().maxconcurrent()); // still capped by max } private static void assertDocumentStoreMaxFileSize(long expFileSizeBytes, int wantedMemoryGb) { diff --git a/configdefinitions/src/vespa/fleetcontroller.def b/configdefinitions/src/vespa/fleetcontroller.def index 10eb408ed69..98b4c3b0216 100644 --- a/configdefinitions/src/vespa/fleetcontroller.def +++ b/configdefinitions/src/vespa/fleetcontroller.def @@ -64,11 +64,6 @@ init_progress_time int default=0 ## we dont change the state too often. min_time_between_new_systemstates int default=10000 -## Sets how many milliseconds to wait between each state poll for old nodes -## requiring state polling. (4.1 or older) -## TODO: Not used, remove in Vespa 9 -state_polling_frequency int default=5000 - ## The maximum amount of premature crashes a node is allowed to have in a row ## before the fleetcontroller disables that node. max_premature_crashes int default=100000 @@ -181,9 +176,6 @@ min_merge_completion_ratio double default=1.0 ## transition logic aims to minimize the window of time where active states diverge. enable_two_phase_cluster_state_transitions bool default=false -## Deprecated - not used -determine_buckets_from_bucket_space_metric bool default=true - # If enabled, the cluster controller observes reported (categorized) resource usage from content nodes (via host info), # and decides whether external feed should be blocked (or unblocked) in the entire cluster. # diff --git a/configserver/src/main/java/com/yahoo/vespa/config/server/deploy/ModelContextImpl.java b/configserver/src/main/java/com/yahoo/vespa/config/server/deploy/ModelContextImpl.java index 7a2377594a1..62431ce4c06 100644 --- a/configserver/src/main/java/com/yahoo/vespa/config/server/deploy/ModelContextImpl.java +++ b/configserver/src/main/java/com/yahoo/vespa/config/server/deploy/ModelContextImpl.java @@ -22,10 +22,10 @@ import com.yahoo.config.provision.AthenzDomain; import com.yahoo.config.provision.CloudAccount; import com.yahoo.config.provision.ClusterSpec; import com.yahoo.config.provision.DockerImage; +import com.yahoo.config.provision.HostName; import com.yahoo.config.provision.TenantName; import com.yahoo.config.provision.Zone; import com.yahoo.container.jdisc.secretstore.SecretStore; -import com.yahoo.config.provision.HostName; import com.yahoo.vespa.config.server.tenant.SecretStoreExternalIdRetriever; import com.yahoo.vespa.flags.FetchVector; import com.yahoo.vespa.flags.FlagSource; @@ -33,7 +33,6 @@ import com.yahoo.vespa.flags.Flags; import com.yahoo.vespa.flags.PermanentFlags; import com.yahoo.vespa.flags.StringFlag; import com.yahoo.vespa.flags.UnboundFlag; - import java.io.File; import java.net.URI; import java.security.cert.X509Certificate; @@ -41,7 +40,7 @@ import java.util.List; import java.util.Optional; import java.util.Set; import java.util.concurrent.ExecutorService; -import java.util.function.ToIntFunction; +import java.util.function.Predicate; import static com.yahoo.config.provision.NodeResources.Architecture; import static com.yahoo.vespa.config.server.ConfigServerSpec.fromConfig; @@ -174,7 +173,7 @@ public class ModelContextImpl implements ModelContext { private final double feedNiceness; private final List<String> allowedAthenzProxyIdentities; private final int maxActivationInhibitedOutOfSyncGroups; - private final ToIntFunction<ClusterSpec.Type> jvmOmitStackTraceInFastThrow; + private final Predicate<ClusterSpec.Type> jvmOmitStackTraceInFastThrow; private final double resourceLimitDisk; private final double resourceLimitMemory; private final double minNodeRatioPerGroup; @@ -204,6 +203,7 @@ public class ModelContextImpl implements ModelContext { private final int heapPercentage; private final boolean enableGlobalPhase; private final String summaryDecodePolicy; + private final Predicate<ClusterSpec.Id> allowMoreThanOneContentGroupDown; public FeatureFlags(FlagSource source, ApplicationId appId, Version version) { this.defaultTermwiseLimit = flagValue(source, appId, version, Flags.DEFAULT_TERM_WISE_LIMIT); @@ -219,7 +219,7 @@ public class ModelContextImpl implements ModelContext { this.mbus_network_threads = flagValue(source, appId, version, Flags.MBUS_NUM_NETWORK_THREADS); this.allowedAthenzProxyIdentities = flagValue(source, appId, version, Flags.ALLOWED_ATHENZ_PROXY_IDENTITIES); this.maxActivationInhibitedOutOfSyncGroups = flagValue(source, appId, version, Flags.MAX_ACTIVATION_INHIBITED_OUT_OF_SYNC_GROUPS); - this.jvmOmitStackTraceInFastThrow = type -> flagValueAsInt(source, appId, version, type, PermanentFlags.JVM_OMIT_STACK_TRACE_IN_FAST_THROW); + this.jvmOmitStackTraceInFastThrow = type -> flagValue(source, appId, version, type, PermanentFlags.JVM_OMIT_STACK_TRACE_IN_FAST_THROW); this.resourceLimitDisk = flagValue(source, appId, version, PermanentFlags.RESOURCE_LIMIT_DISK); this.resourceLimitMemory = flagValue(source, appId, version, PermanentFlags.RESOURCE_LIMIT_MEMORY); this.minNodeRatioPerGroup = flagValue(source, appId, version, Flags.MIN_NODE_RATIO_PER_GROUP); @@ -250,6 +250,7 @@ public class ModelContextImpl implements ModelContext { this.heapPercentage = flagValue(source, appId, version, PermanentFlags.HEAP_SIZE_PERCENTAGE); this.enableGlobalPhase = flagValue(source, appId, version, Flags.ENABLE_GLOBAL_PHASE); this.summaryDecodePolicy = flagValue(source, appId, version, Flags.SUMMARY_DECODE_POLICY); + this.allowMoreThanOneContentGroupDown = clusterId -> flagValue(source, appId, version, clusterId, Flags.ALLOW_MORE_THAN_ONE_CONTENT_GROUP_DOWN); } @Override public int heapSizePercentage() { return heapPercentage; } @@ -270,7 +271,7 @@ public class ModelContextImpl implements ModelContext { @Override public List<String> allowedAthenzProxyIdentities() { return allowedAthenzProxyIdentities; } @Override public int maxActivationInhibitedOutOfSyncGroups() { return maxActivationInhibitedOutOfSyncGroups; } @Override public String jvmOmitStackTraceInFastThrowOption(ClusterSpec.Type type) { - return translateJvmOmitStackTraceInFastThrowIntToString(jvmOmitStackTraceInFastThrow, type); + return translateJvmOmitStackTraceInFastThrowToString(jvmOmitStackTraceInFastThrow, type); } @Override public double resourceLimitDisk() { return resourceLimitDisk; } @Override public double resourceLimitMemory() { return resourceLimitMemory; } @@ -304,6 +305,7 @@ public class ModelContextImpl implements ModelContext { } @Override public boolean useRestrictedDataPlaneBindings() { return useRestrictedDataPlaneBindings; } @Override public boolean enableGlobalPhase() { return enableGlobalPhase; } + @Override public boolean allowMoreThanOneContentGroupDown(ClusterSpec.Id id) { return allowMoreThanOneContentGroupDown.test(id); } private static <V> V flagValue(FlagSource source, ApplicationId appId, Version vespaVersion, UnboundFlag<? extends V, ?, ?> flag) { return flag.bindTo(source) @@ -331,17 +333,21 @@ public class ModelContextImpl implements ModelContext { .boxedValue(); } - static int flagValueAsInt(FlagSource source, - ApplicationId appId, - Version version, - ClusterSpec.Type clusterType, - UnboundFlag<? extends Boolean, ?, ?> flag) { - return flagValue(source, appId, version, clusterType, flag) ? 1 : 0; + private static <V> V flagValue(FlagSource source, + ApplicationId appId, + Version vespaVersion, + ClusterSpec.Id clusterId, + UnboundFlag<? extends V, ?, ?> flag) { + return flag.bindTo(source) + .with(FetchVector.Dimension.APPLICATION_ID, appId.serializedForm()) + .with(FetchVector.Dimension.CLUSTER_ID, clusterId.value()) + .with(FetchVector.Dimension.VESPA_VERSION, vespaVersion.toFullString()) + .boxedValue(); } - private String translateJvmOmitStackTraceInFastThrowIntToString(ToIntFunction<ClusterSpec.Type> function, - ClusterSpec.Type clusterType) { - return function.applyAsInt(clusterType) == 1 ? "" : "-XX:-OmitStackTraceInFastThrow"; + private String translateJvmOmitStackTraceInFastThrowToString(Predicate<ClusterSpec.Type> function, + ClusterSpec.Type clusterType) { + return function.test(clusterType) ? "" : "-XX:-OmitStackTraceInFastThrow"; } } diff --git a/container-search/abi-spec.json b/container-search/abi-spec.json index 36531fbf5e1..50aba749570 100644 --- a/container-search/abi-spec.json +++ b/container-search/abi-spec.json @@ -209,6 +209,7 @@ ], "methods" : [ "public void <init>(byte[])", + "public void <init>(byte[], boolean)", "public int compareTo(com.yahoo.prelude.hitfield.RawBase64)", "public java.lang.String toString()", "public bridge synthetic int compareTo(java.lang.Object)" diff --git a/container-search/src/main/java/com/yahoo/prelude/hitfield/RawBase64.java b/container-search/src/main/java/com/yahoo/prelude/hitfield/RawBase64.java index 2071e43f54c..ada0797ab02 100644 --- a/container-search/src/main/java/com/yahoo/prelude/hitfield/RawBase64.java +++ b/container-search/src/main/java/com/yahoo/prelude/hitfield/RawBase64.java @@ -9,8 +9,13 @@ import java.util.Base64; */ public class RawBase64 implements Comparable<RawBase64> { private final byte[] content; + private final boolean withoutPadding; public RawBase64(byte[] content) { + this(content, false); + } + public RawBase64(byte[] content, boolean withoutPadding) { this.content = content; + this.withoutPadding = withoutPadding; } @Override @@ -20,6 +25,8 @@ public class RawBase64 implements Comparable<RawBase64> { @Override public String toString() { - return Base64.getEncoder().encodeToString(content); + return withoutPadding + ? Base64.getEncoder().withoutPadding().encodeToString(content) + : Base64.getEncoder().encodeToString(content); } } diff --git a/container-search/src/main/java/com/yahoo/search/grouping/result/RawBucketId.java b/container-search/src/main/java/com/yahoo/search/grouping/result/RawBucketId.java index 9576f548f4a..129c6aadee8 100644 --- a/container-search/src/main/java/com/yahoo/search/grouping/result/RawBucketId.java +++ b/container-search/src/main/java/com/yahoo/search/grouping/result/RawBucketId.java @@ -2,6 +2,7 @@ package com.yahoo.search.grouping.result; import java.util.Arrays; +import java.util.Base64; /** * This class is used in {@link Group} instances where the identifying @@ -18,6 +19,8 @@ public class RawBucketId extends BucketGroupId<byte[]> { * @param to The identifying exclusive-to raw buffer. */ public RawBucketId(byte[] from, byte[] to) { - super("raw_bucket", from, Arrays.toString(from), to, Arrays.toString(to)); + super("raw_bucket", + from, Base64.getEncoder().withoutPadding().encodeToString(from), + to, Base64.getEncoder().withoutPadding().encodeToString(to)); } } diff --git a/container-search/src/main/java/com/yahoo/search/grouping/result/RawId.java b/container-search/src/main/java/com/yahoo/search/grouping/result/RawId.java index de711d0c218..f160f9b66af 100644 --- a/container-search/src/main/java/com/yahoo/search/grouping/result/RawId.java +++ b/container-search/src/main/java/com/yahoo/search/grouping/result/RawId.java @@ -1,7 +1,7 @@ // Copyright Yahoo. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. package com.yahoo.search.grouping.result; -import java.util.Arrays; +import java.util.Base64; /** * This class is used in {@link Group} instances where the identifying expression evaluated to a {@link Byte} array. @@ -16,6 +16,6 @@ public class RawId extends ValueGroupId<byte[]> { * @param value The identifying byte array. */ public RawId(byte[] value) { - super("raw", value, Arrays.toString(value)); + super("raw", value, Base64.getEncoder().withoutPadding().encodeToString(value)); } } diff --git a/container-search/src/main/java/com/yahoo/search/grouping/vespa/ResultBuilder.java b/container-search/src/main/java/com/yahoo/search/grouping/vespa/ResultBuilder.java index 7f006b098cd..2333a180690 100644 --- a/container-search/src/main/java/com/yahoo/search/grouping/vespa/ResultBuilder.java +++ b/container-search/src/main/java/com/yahoo/search/grouping/vespa/ResultBuilder.java @@ -1,6 +1,7 @@ // Copyright Yahoo. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. package com.yahoo.search.grouping.vespa; +import com.yahoo.prelude.hitfield.RawBase64; import com.yahoo.search.grouping.Continuation; import com.yahoo.search.grouping.GroupingRequest; import com.yahoo.search.grouping.result.BoolId; @@ -28,6 +29,7 @@ import com.yahoo.searchlib.aggregation.Hit; import com.yahoo.searchlib.aggregation.HitsAggregationResult; import com.yahoo.searchlib.aggregation.MaxAggregationResult; import com.yahoo.searchlib.aggregation.MinAggregationResult; +import com.yahoo.searchlib.aggregation.RawData; import com.yahoo.searchlib.aggregation.StandardDeviationAggregationResult; import com.yahoo.searchlib.aggregation.SumAggregationResult; import com.yahoo.searchlib.aggregation.XorAggregationResult; @@ -169,7 +171,7 @@ class ResultBuilder { } else { String label = transform.getLabel(result.getTag()); if (label != null) { - group.setField(label, newResult(result, tag)); + group.setField(label, convertResult(newResult(result, tag))); } } } @@ -228,24 +230,27 @@ class ResultBuilder { return new RawId(res.getRaw()); } else if (res instanceof StringResultNode) { return new StringId(res.getString()); - } else if (res instanceof FloatBucketResultNode) { - FloatBucketResultNode bucketId = (FloatBucketResultNode)res; + } else if (res instanceof FloatBucketResultNode bucketId) { return new DoubleBucketId(bucketId.getFrom(), bucketId.getTo()); - } else if (res instanceof IntegerBucketResultNode) { - IntegerBucketResultNode bucketId = (IntegerBucketResultNode)res; + } else if (res instanceof IntegerBucketResultNode bucketId) { return new LongBucketId(bucketId.getFrom(), bucketId.getTo()); - } else if (res instanceof StringBucketResultNode) { - StringBucketResultNode bucketId = (StringBucketResultNode)res; + } else if (res instanceof StringBucketResultNode bucketId) { return new StringBucketId(bucketId.getFrom(), bucketId.getTo()); - } else if (res instanceof RawBucketResultNode) { - RawBucketResultNode bucketId = (RawBucketResultNode)res; + } else if (res instanceof RawBucketResultNode bucketId) { return new RawBucketId(bucketId.getFrom(), bucketId.getTo()); } else { throw new UnsupportedOperationException(res.getClass().getName()); } } - Object newResult(ExpressionNode execResult, int tag) { + private Object convertResult(Object value) { + if (value instanceof RawData raw) { + return new RawBase64(raw.getData(), true); + } + return value; + } + + private Object newResult(ExpressionNode execResult, int tag) { if (execResult instanceof AverageAggregationResult) { return ((AverageAggregationResult)execResult).getAverage().getNumber(); } else if (execResult instanceof CountAggregationResult) { diff --git a/container-search/src/main/java/com/yahoo/search/rendering/JsonRenderer.java b/container-search/src/main/java/com/yahoo/search/rendering/JsonRenderer.java index b36c8788877..500227e2607 100644 --- a/container-search/src/main/java/com/yahoo/search/rendering/JsonRenderer.java +++ b/container-search/src/main/java/com/yahoo/search/rendering/JsonRenderer.java @@ -57,7 +57,7 @@ import java.math.BigDecimal; import java.math.BigInteger; import java.nio.charset.StandardCharsets; import java.util.ArrayDeque; -import java.util.Arrays; +import java.util.Base64; import java.util.Deque; import java.util.Map; import java.util.Optional; @@ -420,9 +420,9 @@ public class JsonRenderer extends AsynchronousSectionedRenderer<Result> { } protected void renderGroupMetadata(GroupId id) throws IOException { - if (!(id instanceof ValueGroupId || id instanceof BucketGroupId)) return; + if (!(id instanceof ValueGroupId<?> || id instanceof BucketGroupId)) return; - if (id instanceof ValueGroupId valueId) { + if (id instanceof ValueGroupId<?> valueId) { generator.writeStringField(GROUPING_VALUE, getIdValue(valueId)); } else { BucketGroupId<?> bucketId = (BucketGroupId<?>) id; @@ -434,15 +434,21 @@ public class JsonRenderer extends AsynchronousSectionedRenderer<Result> { } private static String getIdValue(ValueGroupId<?> id) { - return (id instanceof RawId ? Arrays.toString(((RawId) id).getValue()) : id.getValue()).toString(); + return (id instanceof RawId raw) + ? Base64.getEncoder().withoutPadding().encodeToString(raw.getValue()) + : id.getValue().toString(); } private static String getBucketFrom(BucketGroupId<?> id) { - return (id instanceof RawBucketId ? Arrays.toString(((RawBucketId) id).getFrom()) : id.getFrom()).toString(); + if (id instanceof RawBucketId rawBucketId) + return Base64.getEncoder().withoutPadding().encodeToString(rawBucketId.getFrom()); + return id.getFrom().toString(); } private static String getBucketTo(BucketGroupId<?> id) { - return (id instanceof RawBucketId ? Arrays.toString(((RawBucketId) id).getTo()) : id.getTo()).toString(); + if (id instanceof RawBucketId rawBucketId) + return Base64.getEncoder().withoutPadding().encodeToString(rawBucketId.getTo()); + return id.getTo().toString(); } protected void renderTotalHitCount(Hit hit) throws IOException { diff --git a/container-search/src/test/java/com/yahoo/search/grouping/result/GroupIdTestCase.java b/container-search/src/test/java/com/yahoo/search/grouping/result/GroupIdTestCase.java index 7b2f0d52742..f986c593fae 100644 --- a/container-search/src/test/java/com/yahoo/search/grouping/result/GroupIdTestCase.java +++ b/container-search/src/test/java/com/yahoo/search/grouping/result/GroupIdTestCase.java @@ -47,8 +47,8 @@ public class GroupIdTestCase { assertEquals("group:long:69", new LongId(69L).toString()); assertEquals("group:long_bucket:6:9", new LongBucketId(6L, 9L).toString()); assertEquals("group:null", new NullId().toString()); - assertEquals("group:raw:[6, 9]", new RawId(new byte[]{6, 9}).toString()); - assertEquals("group:raw_bucket:[6, 9]:[9, 6]", new RawBucketId(new byte[]{6, 9}, new byte[]{9, 6}).toString()); + assertEquals("group:raw:Bgk", new RawId(new byte[]{6, 9}).toString()); + assertEquals("group:raw_bucket:Bgk:CQY", new RawBucketId(new byte[]{6, 9}, new byte[]{9, 6}).toString()); assertTrue(new RootId(0).toString().startsWith("group:root:")); assertEquals("group:string:69", new StringId("69").toString()); assertEquals("group:string_bucket:6:9", new StringBucketId("6", "9").toString()); diff --git a/container-search/src/test/java/com/yahoo/search/grouping/vespa/ResultBuilderTestCase.java b/container-search/src/test/java/com/yahoo/search/grouping/vespa/ResultBuilderTestCase.java index 019a022b7e6..b0b48bb8731 100644 --- a/container-search/src/test/java/com/yahoo/search/grouping/vespa/ResultBuilderTestCase.java +++ b/container-search/src/test/java/com/yahoo/search/grouping/vespa/ResultBuilderTestCase.java @@ -10,12 +10,39 @@ import com.yahoo.search.grouping.result.GroupList; import com.yahoo.search.grouping.result.HitList; import com.yahoo.search.result.HitGroup; import com.yahoo.search.result.Relevance; -import com.yahoo.searchlib.aggregation.*; +import com.yahoo.searchlib.aggregation.AggregationResult; +import com.yahoo.searchlib.aggregation.AverageAggregationResult; +import com.yahoo.searchlib.aggregation.CountAggregationResult; +import com.yahoo.searchlib.aggregation.ExpressionCountAggregationResult; +import com.yahoo.searchlib.aggregation.FS4Hit; +import com.yahoo.searchlib.aggregation.Group; +import com.yahoo.searchlib.aggregation.Grouping; +import com.yahoo.searchlib.aggregation.HitsAggregationResult; +import com.yahoo.searchlib.aggregation.MaxAggregationResult; +import com.yahoo.searchlib.aggregation.MinAggregationResult; +import com.yahoo.searchlib.aggregation.SumAggregationResult; +import com.yahoo.searchlib.aggregation.XorAggregationResult; import com.yahoo.searchlib.aggregation.hll.SparseSketch; -import com.yahoo.searchlib.expression.*; +import com.yahoo.searchlib.expression.FloatBucketResultNode; +import com.yahoo.searchlib.expression.FloatResultNode; +import com.yahoo.searchlib.expression.IntegerBucketResultNode; +import com.yahoo.searchlib.expression.IntegerResultNode; +import com.yahoo.searchlib.expression.NullResultNode; +import com.yahoo.searchlib.expression.RawBucketResultNode; +import com.yahoo.searchlib.expression.RawResultNode; +import com.yahoo.searchlib.expression.ResultNode; +import com.yahoo.searchlib.expression.StringBucketResultNode; +import com.yahoo.searchlib.expression.StringResultNode; import org.junit.jupiter.api.Test; -import java.util.*; + +import java.util.Arrays; +import java.util.Collections; +import java.util.LinkedHashMap; +import java.util.LinkedList; +import java.util.List; +import java.util.Map; +import java.util.TreeMap; import static org.junit.jupiter.api.Assertions.assertEquals; import static org.junit.jupiter.api.Assertions.fail; @@ -33,19 +60,19 @@ public class ResultBuilderTestCase { assertGroupId("group:6.9", new FloatResultNode(6.9)); assertGroupId("group:69", new IntegerResultNode(69)); assertGroupId("group:null", new NullResultNode()); - assertGroupId("group:[6, 9]", new RawResultNode(new byte[]{6, 9})); + assertGroupId("group:Bgk", new RawResultNode(new byte[]{6, 9})); assertGroupId("group:a", new StringResultNode("a")); assertGroupId("group:6.9:9.6", new FloatBucketResultNode(6.9, 9.6)); assertGroupId("group:6:9", new IntegerBucketResultNode(6, 9)); assertGroupId("group:a:b", new StringBucketResultNode("a", "b")); - assertGroupId("group:[6, 9]:[9, 6]", new RawBucketResultNode(new RawResultNode(new byte[]{6, 9}), + assertGroupId("group:Bgk:CQY", new RawBucketResultNode(new RawResultNode(new byte[]{6, 9}), new RawResultNode(new byte[]{9, 6}))); } @Test void requireThatUnknownGroupIdThrows() { assertBuildFail("all(group(a) each(output(count())))", - Arrays.asList(newGrouping(new Group().setTag(2).setId(new MyResultNode()))), + List.of(newGrouping(new Group().setTag(2).setId(new MyResultNode()))), "com.yahoo.search.grouping.vespa.ResultBuilderTestCase$MyResultNode"); } @@ -61,9 +88,17 @@ public class ResultBuilderTestCase { } @Test + void requireThatAllBasicResultsCanBeConverted() { + assertResult("69", new MinAggregationResult(new IntegerResultNode(69))); + assertResult("69.3", new MinAggregationResult(new FloatResultNode(69.3))); + assertResult("69.6", new MinAggregationResult(new StringResultNode("69.6"))); + assertResult("Bgk", new MinAggregationResult(new RawResultNode(new byte[]{6,9}))); + } + + @Test void requireThatUnknownExpressionNodeThrows() { assertBuildFail("all(group(a) each(output(count())))", - Arrays.asList(newGrouping(newGroup(2, 2, new MyAggregationResult().setTag(3)))), + List.of(newGrouping(newGroup(2, 2, new MyAggregationResult().setTag(3)))), "com.yahoo.search.grouping.vespa.ResultBuilderTestCase$MyAggregationResult"); } @@ -127,10 +162,10 @@ public class ResultBuilderTestCase { @Test void requireThatParallelResultsAreTransformed() { assertBuild("all(group(foo) each(output(count())) as(bar) each(output(count())) as(baz))", - Arrays.asList(new Grouping().setRoot(newGroup(1, 0)), + List.of(new Grouping().setRoot(newGroup(1, 0)), new Grouping().setRoot(newGroup(1, 0)))); assertBuildFail("all(group(foo) each(output(count())) as(bar) each(output(count())) as(baz))", - Arrays.asList(new Grouping().setRoot(newGroup(2)), + List.of(new Grouping().setRoot(newGroup(2)), new Grouping().setRoot(newGroup(3))), "Expected 1 group, got 2."); } @@ -138,15 +173,15 @@ public class ResultBuilderTestCase { @Test void requireThatTagsAreHandledCorrectly() { assertBuild("all(group(a) each(output(count())))", - Arrays.asList(newGrouping( + List.of(newGrouping( newGroup(7, new CountAggregationResult(0))))); } @Test void requireThatEmptyBranchesArePruned() { - assertBuildFail("all()", Collections.<Grouping>emptyList(), "Expected 1 group, got 0."); - assertBuildFail("all(group(a))", Collections.<Grouping>emptyList(), "Expected 1 group, got 0."); - assertBuildFail("all(group(a) each())", Collections.<Grouping>emptyList(), "Expected 1 group, got 0."); + assertBuildFail("all()", List.of(), "Expected 1 group, got 0."); + assertBuildFail("all(group(a))", List.of(), "Expected 1 group, got 0."); + assertBuildFail("all(group(a) each())", List.of(), "Expected 1 group, got 0."); Grouping grouping = newGrouping(newGroup(2, new CountAggregationResult(69).setTag(3))); String expectedOutput = "RootGroup{id=group:root}[GroupList{label=a}[Group{id=group:2, count()=69}[]]]"; @@ -189,14 +224,14 @@ public class ResultBuilderTestCase { "HitList{label=bar}[Hit{id=hit:1}, Hit{id=hit:2}]]]]"); assertLayout("all(group(foo) each(each(output(summary())) as(bar)" + " each(output(summary())) as(baz)))", - Arrays.asList(newGrouping(newGroup(2, newHitList(3, 2))), + List.of(newGrouping(newGroup(2, newHitList(3, 2))), newGrouping(newGroup(2, newHitList(4, 2)))), "RootGroup{id=group:root}[GroupList{label=foo}[Group{id=group:2}[" + "HitList{label=bar}[Hit{id=hit:1}, Hit{id=hit:2}], " + "HitList{label=baz}[Hit{id=hit:1}, Hit{id=hit:2}]]]]"); assertLayout("all(group(foo) each(each(output(summary())))" + " each(each(output(summary()))) as(bar))", - Arrays.asList(newGrouping(newGroup(2, newHitList(3, 2))), + List.of(newGrouping(newGroup(2, newHitList(3, 2))), newGrouping(newGroup(4, newHitList(5, 2)))), "RootGroup{id=group:root}[" + "GroupList{label=foo}[Group{id=group:2}[HitList{label=hits}[Hit{id=hit:1}, Hit{id=hit:2}]]], " + @@ -273,18 +308,18 @@ public class ResultBuilderTestCase { assertResultCont("all(group(a) max(2) each(output(count())) as(foo)" + " each(output(count())) as(bar))", - Arrays.asList(newGrouping(newGroup(2, 1, new CountAggregationResult(1))), + List.of(newGrouping(newGroup(2, 1, new CountAggregationResult(1))), newGrouping(newGroup(4, 2, new CountAggregationResult(4)))), "[]"); assertResultCont("all(group(a) max(2) each(output(count())) as(foo)" + " each(output(count())) as(bar))", - Arrays.asList(newGrouping(newGroup(2, 1, new CountAggregationResult(1))), + List.of(newGrouping(newGroup(2, 1, new CountAggregationResult(1))), newGrouping(newGroup(4, 2, new CountAggregationResult(4)))), newOffset(newResultId(0), 2, 1), "[0=1]"); assertResultCont("all(group(a) max(2) each(output(count())) as(foo)" + " each(output(count())) as(bar))", - Arrays.asList(newGrouping(newGroup(2, 1, new CountAggregationResult(1))), + List.of(newGrouping(newGroup(2, 1, new CountAggregationResult(1))), newGrouping(newGroup(4, 2, new CountAggregationResult(4)))), newComposite(newOffset(newResultId(0), 2, 2), newOffset(newResultId(1), 4, 1)), @@ -299,18 +334,18 @@ public class ResultBuilderTestCase { assertResultCont("all(group(a) each(max(2) each(output(summary()))) as(foo)" + " each(max(2) each(output(summary()))) as(bar))", - Arrays.asList(newGrouping(newGroup(2, newHitList(3, 4))), + List.of(newGrouping(newGroup(2, newHitList(3, 4))), newGrouping(newGroup(4, newHitList(5, 4)))), "[]"); assertResultCont("all(group(a) each(max(2) each(output(summary()))) as(foo)" + " each(max(2) each(output(summary()))) as(bar))", - Arrays.asList(newGrouping(newGroup(2, newHitList(3, 4))), + List.of(newGrouping(newGroup(2, newHitList(3, 4))), newGrouping(newGroup(4, newHitList(5, 4)))), newOffset(newResultId(0, 0, 0), 3, 1), "[0.0.0=1]"); assertResultCont("all(group(a) each(max(2) each(output(summary()))) as(foo)" + " each(max(2) each(output(summary()))) as(bar))", - Arrays.asList(newGrouping(newGroup(2, newHitList(3, 4))), + List.of(newGrouping(newGroup(2, newHitList(3, 4))), newGrouping(newGroup(4, newHitList(5, 4)))), newComposite(newOffset(newResultId(0, 0, 0), 3, 2), newOffset(newResultId(1, 0, 0), 5, 1)), @@ -404,7 +439,7 @@ public class ResultBuilderTestCase { void requireThatGroupListContinuationsCanBeSetInSiblingGroupLists() { String request = "all(group(a) max(2) each(output(count())) as(foo)" + " each(output(count())) as(bar))"; - List<Grouping> result = Arrays.asList(newGrouping(newGroup(2, 1, new CountAggregationResult(1)), + List<Grouping> result = List.of(newGrouping(newGroup(2, 1, new CountAggregationResult(1)), newGroup(2, 2, new CountAggregationResult(2)), newGroup(2, 3, new CountAggregationResult(3)), newGroup(2, 4, new CountAggregationResult(4))), @@ -646,7 +681,7 @@ public class ResultBuilderTestCase { void requireThatHitListContinuationsCanBeSetInSiblingHitLists() { String request = "all(group(a) each(max(2) each(output(summary()))) as(foo)" + " each(max(2) each(output(summary()))) as(bar))"; - List<Grouping> result = Arrays.asList(newGrouping(newGroup(2, newHitList(3, 4))), + List<Grouping> result = List.of(newGrouping(newGroup(2, newHitList(3, 4))), newGrouping(newGroup(4, newHitList(5, 4)))); assertContinuation(request, result, newComposite(newOffset(newResultId(0, 0, 0), 3, 0), newOffset(newResultId(1, 0, 0), 5, 5)), @@ -839,7 +874,7 @@ public class ResultBuilderTestCase { } private static void assertResultCont(String request, Grouping result, Continuation cont, String expected) { - assertOutput(request, Arrays.asList(result), cont, new ResultContWriter(), expected); + assertOutput(request, List.of(result), cont, new ResultContWriter(), expected); } private static void assertResultCont(String request, List<Grouping> result, String expected) { @@ -851,11 +886,11 @@ public class ResultBuilderTestCase { } private static void assertContinuation(String request, Grouping result, String expected) { - assertOutput(request, Arrays.asList(result), null, new ContinuationWriter(), expected); + assertOutput(request, List.of(result), null, new ContinuationWriter(), expected); } private static void assertContinuation(String request, Grouping result, Continuation cont, String expected) { - assertOutput(request, Arrays.asList(result), cont, new ContinuationWriter(), expected); + assertOutput(request, List.of(result), cont, new ContinuationWriter(), expected); } private static void assertContinuation(String request, List<Grouping> result, Continuation cont, String expected) { @@ -863,7 +898,7 @@ public class ResultBuilderTestCase { } private static void assertLayout(String request, Grouping result, String expected) { - assertOutput(request, Arrays.asList(result), null, new LayoutWriter(), expected); + assertOutput(request, List.of(result), null, new LayoutWriter(), expected); } private static void assertLayout(String request, List<Grouping> result, String expected) { @@ -953,8 +988,7 @@ public class ResultBuilderTestCase { } String toString(Continuation cnt) { - if (cnt instanceof OffsetContinuation) { - OffsetContinuation off = (OffsetContinuation)cnt; + if (cnt instanceof OffsetContinuation off) { String id = off.getResultId().toString().replace(", ", "."); return id.substring(5, id.length() - 1) + "=" + off.getOffset(); } else if (cnt instanceof CompositeContinuation) { diff --git a/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/integration/deployment/ApplicationVersion.java b/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/integration/deployment/ApplicationVersion.java index 04604ae7007..eb2005bf268 100644 --- a/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/integration/deployment/ApplicationVersion.java +++ b/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/integration/deployment/ApplicationVersion.java @@ -1,11 +1,9 @@ // Copyright Yahoo. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. package com.yahoo.vespa.hosted.controller.api.integration.deployment; -import ai.vespa.validation.Validation; import com.yahoo.component.Version; import java.time.Instant; -import java.util.Objects; import java.util.Optional; import java.util.OptionalLong; @@ -33,6 +31,7 @@ public class ApplicationVersion implements Comparable<ApplicationVersion> { private final Optional<String> sourceUrl; private final Optional<String> commit; private final Optional<String> bundleHash; + private final Optional<Instant> obsoleteAt; private final boolean hasPackage; private final boolean shouldSkip; private final Optional<String> description; @@ -41,7 +40,7 @@ public class ApplicationVersion implements Comparable<ApplicationVersion> { public ApplicationVersion(RevisionId id, Optional<SourceRevision> source, Optional<String> authorEmail, Optional<Version> compileVersion, Optional<Integer> allowedMajor, Optional<Instant> buildTime, Optional<String> sourceUrl, Optional<String> commit, Optional<String> bundleHash, - boolean hasPackage, boolean shouldSkip, Optional<String> description, int risk) { + Optional<Instant> obsoleteAt, boolean hasPackage, boolean shouldSkip, Optional<String> description, int risk) { if (commit.isPresent() && commit.get().length() > 128) throw new IllegalArgumentException("Commit may not be longer than 128 characters"); @@ -61,6 +60,7 @@ public class ApplicationVersion implements Comparable<ApplicationVersion> { this.sourceUrl = requireNonNull(sourceUrl, "sourceUrl cannot be null"); this.commit = requireNonNull(commit, "commit cannot be null"); this.bundleHash = bundleHash; + this.obsoleteAt = obsoleteAt; this.hasPackage = hasPackage; this.shouldSkip = shouldSkip; this.description = description; @@ -71,19 +71,9 @@ public class ApplicationVersion implements Comparable<ApplicationVersion> { return id; } - /** Create an application package version from a completed build, without an author email */ - public static ApplicationVersion from(RevisionId id, SourceRevision source) { - return new ApplicationVersion(id, Optional.of(source), Optional.empty(), Optional.empty(), Optional.empty(), Optional.empty(), Optional.empty(), Optional.empty(), Optional.empty(), true, false, Optional.empty(), 0); - } - - /** Creates a version from a completed build, an author email, and build metadata. */ - public static ApplicationVersion from(RevisionId id, SourceRevision source, String authorEmail, Version compileVersion, Instant buildTime) { - return new ApplicationVersion(id, Optional.of(source), Optional.of(authorEmail), Optional.of(compileVersion), Optional.empty(), Optional.of(buildTime), Optional.empty(), Optional.empty(), Optional.empty(), true, false, Optional.empty(), 0); - } - /** Creates a minimal version for a development build. */ public static ApplicationVersion forDevelopment(RevisionId id, Optional<Version> compileVersion, Optional<Integer> allowedMajor) { - return new ApplicationVersion(id, Optional.empty(), Optional.empty(), compileVersion, allowedMajor, Optional.empty(), Optional.empty(), Optional.empty(), Optional.empty(), true, false, Optional.empty(), 0); + return new ApplicationVersion(id, Optional.empty(), Optional.empty(), compileVersion, allowedMajor, Optional.empty(), Optional.empty(), Optional.empty(), Optional.empty(), Optional.empty(), true, false, Optional.empty(), 0); } /** Creates a version from a completed build, an author email, and build metadata. */ @@ -91,7 +81,7 @@ public class ApplicationVersion implements Comparable<ApplicationVersion> { Optional<Version> compileVersion, Optional<Integer> allowedMajor, Optional<Instant> buildTime, Optional<String> sourceUrl, Optional<String> commit, Optional<String> bundleHash, Optional<String> description, int risk) { return new ApplicationVersion(id, source, authorEmail, compileVersion, allowedMajor, buildTime, - sourceUrl, commit, bundleHash, true, false, description, risk); + sourceUrl, commit, bundleHash, Optional.empty(), true, false, description, risk); } /** Returns a unique identifier for this version or "unknown" if version is not known */ @@ -150,7 +140,17 @@ public class ApplicationVersion implements Comparable<ApplicationVersion> { /** Returns a copy of this without a package stored. */ public ApplicationVersion withoutPackage() { - return new ApplicationVersion(id, source, authorEmail, compileVersion, allowedMajor, buildTime, sourceUrl, commit, bundleHash, false, shouldSkip, description, risk); + return new ApplicationVersion(id, source, authorEmail, compileVersion, allowedMajor, buildTime, sourceUrl, commit, bundleHash, obsoleteAt, false, shouldSkip, description, risk); + } + + /** Returns a copy of this which is obsolete now. */ + public ApplicationVersion obsoleteAt(Instant now) { + return new ApplicationVersion(id, source, authorEmail, compileVersion, allowedMajor, buildTime, sourceUrl, commit, bundleHash, Optional.of(now), hasPackage, shouldSkip, description, risk); + } + + /** Returns the instant at which this became obsolete, i.e., no longer relevant for automated deployments. */ + public Optional<Instant> obsoleteAt() { + return obsoleteAt; } /** Whether we still have the package for this revision. */ @@ -160,7 +160,7 @@ public class ApplicationVersion implements Comparable<ApplicationVersion> { /** Returns a copy of this which will not be rolled out to production. */ public ApplicationVersion skipped() { - return new ApplicationVersion(id, source, authorEmail, compileVersion, allowedMajor, buildTime, sourceUrl, commit, bundleHash, hasPackage, true, description, risk); + return new ApplicationVersion(id, source, authorEmail, compileVersion, allowedMajor, buildTime, sourceUrl, commit, bundleHash, obsoleteAt, hasPackage, true, description, risk); } /** Whether we still have the package for this revision. */ diff --git a/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/integration/vcmr/HostAction.java b/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/integration/vcmr/HostAction.java index f3bee721343..85c7f78eabc 100644 --- a/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/integration/vcmr/HostAction.java +++ b/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/integration/vcmr/HostAction.java @@ -67,6 +67,7 @@ public class HostAction { OUT_OF_SYNC, NONE, RETIRING, + READY, RETIRED, COMPLETE } diff --git a/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/integration/vcmr/VcmrReport.java b/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/integration/vcmr/VcmrReport.java index 969e6fb1e01..660f3c50556 100644 --- a/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/integration/vcmr/VcmrReport.java +++ b/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/integration/vcmr/VcmrReport.java @@ -11,7 +11,6 @@ import java.time.ZonedDateTime; import java.util.HashMap; import java.util.HashSet; import java.util.Map; -import java.util.Objects; import java.util.Set; import static com.yahoo.yolean.Exceptions.uncheck; @@ -47,18 +46,18 @@ public class VcmrReport { /** * @return true if list of VCMRs is changed */ - public boolean addVcmr(String id, ZonedDateTime plannedStartTime, ZonedDateTime plannedEndtime) { - var vcmr = new Vcmr(id, plannedStartTime, plannedEndtime); + public boolean addVcmr(ChangeRequestSource source) { + var vcmr = new Vcmr(source.getId(), source.getStatus().name(), source.getPlannedStartTime(), source.getPlannedEndTime()); if (vcmrs.contains(vcmr)) return false; // Remove to catch any changes in start/end time - removeVcmr(id); + removeVcmr(source.getId()); return vcmrs.add(vcmr); } public boolean removeVcmr(String id) { - return vcmrs.removeIf(vcmr -> id.equals(vcmr.getId())); + return vcmrs.removeIf(vcmr -> id.equals(vcmr.id())); } public static String getReportId() { @@ -93,55 +92,9 @@ public class VcmrReport { return "VCMRReport{" + vcmrs + "}"; } - public static class Vcmr { - - private String id; - private ZonedDateTime plannedStartTime; - private ZonedDateTime plannedEndTime; - - Vcmr(@JsonProperty("id") String id, - @JsonProperty("plannedStartTime") ZonedDateTime plannedStartTime, - @JsonProperty("plannedEndTime") ZonedDateTime plannedEndTime) { - this.id = id; - this.plannedStartTime = plannedStartTime; - this.plannedEndTime = plannedEndTime; - } - - public String getId() { - return id; - } - - public ZonedDateTime getPlannedStartTime() { - return plannedStartTime; - } - - public ZonedDateTime getPlannedEndTime() { - return plannedEndTime; - } - - @Override - public boolean equals(Object o) { - if (this == o) return true; - if (o == null || getClass() != o.getClass()) return false; - Vcmr vcmr = (Vcmr) o; - return Objects.equals(id, vcmr.id) && - Objects.equals(plannedStartTime, vcmr.plannedStartTime) && - Objects.equals(plannedEndTime, vcmr.plannedEndTime); - } - - @Override - public int hashCode() { - return Objects.hash(id, plannedStartTime, plannedEndTime); - } - - @Override - public String toString() { - return "VCMR{" + - "id='" + id + '\'' + - ", plannedStartTime=" + plannedStartTime + - ", plannedEndTime=" + plannedEndTime + - '}'; - } - } + public record Vcmr (@JsonProperty("id") String id, + @JsonProperty("status") String status, + @JsonProperty("plannedStartTime") ZonedDateTime plannedStartTime, + @JsonProperty("plannedEndTime") ZonedDateTime plannedEndTime) {} } diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/application/Change.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/application/Change.java index 64cad599168..5ebb3d53529 100644 --- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/application/Change.java +++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/application/Change.java @@ -2,7 +2,6 @@ package com.yahoo.vespa.hosted.controller.application; import com.yahoo.component.Version; -import com.yahoo.vespa.hosted.controller.api.integration.deployment.ApplicationVersion; import com.yahoo.vespa.hosted.controller.api.integration.deployment.RevisionId; import java.util.Objects; @@ -23,7 +22,7 @@ import static java.util.Objects.requireNonNull; */ public final class Change { - private static final Change empty = new Change(Optional.empty(), Optional.empty(), false); + private static final Change empty = new Change(Optional.empty(), Optional.empty(), false, false); /** The platform version we are upgrading to, or empty if none */ private final Optional<Version> platform; @@ -32,23 +31,27 @@ public final class Change { private final Optional<RevisionId> revision; /** Whether this change is a pin to its contained Vespa version, or to the application's current. */ - private final boolean pinned; + private final boolean platformPinned; - private Change(Optional<Version> platform, Optional<RevisionId> revision, boolean pinned) { + /** Whether this change is a pin to its contained application revision, or to the application's current. */ + private final boolean revisionPinned; + + private Change(Optional<Version> platform, Optional<RevisionId> revision, boolean platformPinned, boolean revisionPinned) { this.platform = requireNonNull(platform, "platform cannot be null"); this.revision = requireNonNull(revision, "revision cannot be null"); if (revision.isPresent() && ( ! revision.get().isProduction())) { throw new IllegalArgumentException("Application version to deploy must be a known version"); } - this.pinned = pinned; + this.platformPinned = platformPinned; + this.revisionPinned = revisionPinned; } public Change withoutPlatform() { - return new Change(Optional.empty(), revision, pinned); + return new Change(Optional.empty(), revision, platformPinned, revisionPinned); } public Change withoutApplication() { - return new Change(platform, Optional.empty(), pinned); + return new Change(platform, Optional.empty(), platformPinned, revisionPinned); } /** Returns whether a change should currently be deployed */ @@ -58,7 +61,7 @@ public final class Change { /** Returns whether this is the empty change. */ public boolean isEmpty() { - return ! hasTargets() && ! pinned; + return ! hasTargets() && ! platformPinned && ! revisionPinned; } /** Returns the platform version carried by this. */ @@ -67,42 +70,55 @@ public final class Change { /** Returns the application version carried by this. */ public Optional<RevisionId> revision() { return revision; } - public boolean isPinned() { return pinned; } + public boolean isPlatformPinned() { return platformPinned; } + + public boolean isRevisionPinned() { return revisionPinned; } /** Returns an instance representing no change */ public static Change empty() { return empty; } /** Returns a version of this change which replaces or adds this platform change */ public Change with(Version platformVersion) { - if (pinned) + if (platformPinned) throw new IllegalArgumentException("Not allowed to set a platform version when pinned."); - return new Change(Optional.of(platformVersion), revision, pinned); + return new Change(Optional.of(platformVersion), revision, platformPinned, revisionPinned); } /** Returns a version of this change which replaces or adds this revision change */ public Change with(RevisionId revision) { - return new Change(platform, Optional.of(revision), pinned); + if (revisionPinned) + throw new IllegalArgumentException("Not allowed to set a revision when pinned."); + + return new Change(platform, Optional.of(revision), platformPinned, revisionPinned); + } + + /** Returns a change with the versions of this, and with the platform version pinned. */ + public Change withPlatformPin() { + return new Change(platform, revision, true, revisionPinned); + } + + /** Returns a change with the versions of this, and with the platform version unpinned. */ + public Change withoutPlatformPin() { + return new Change(platform, revision, false, revisionPinned); } /** Returns a change with the versions of this, and with the platform version pinned. */ - public Change withPin() { - return new Change(platform, revision, true); + public Change withRevisionPin() { + return new Change(platform, revision, platformPinned, true); } /** Returns a change with the versions of this, and with the platform version unpinned. */ - public Change withoutPin() { - return new Change(platform, revision, false); + public Change withoutRevisionPin() { + return new Change(platform, revision, platformPinned, false); } /** Returns the change obtained when overwriting elements of the given change with any present in this */ public Change onTopOf(Change other) { - if (platform.isPresent()) - other = other.with(platform.get()); - if (revision.isPresent()) - other = other.with(revision.get()); - if (pinned) - other = other.withPin(); + if (platform.isPresent()) other = other.with(platform.get()); + if (revision.isPresent()) other = other.with(revision.get()); + if (platformPinned) other = other.withPlatformPin(); + if (revisionPinned) other = other.withRevisionPin(); return other; } @@ -111,34 +127,38 @@ public final class Change { if (this == o) return true; if (!(o instanceof Change)) return false; Change change = (Change) o; - return pinned == change.pinned && + return platformPinned == change.platformPinned && + revisionPinned == change.revisionPinned && Objects.equals(platform, change.platform) && Objects.equals(revision, change.revision); } @Override public int hashCode() { - return Objects.hash(platform, revision, pinned); + return Objects.hash(platform, revision, platformPinned, revisionPinned); } @Override public String toString() { StringJoiner changes = new StringJoiner(" and "); - if (pinned) + if (platformPinned) changes.add("pin to " + platform.map(Version::toString).orElse("current platform")); else platform.ifPresent(version -> changes.add("upgrade to " + version)); - revision.ifPresent(revision -> changes.add("revision change to " + revision)); + if (revisionPinned) + changes.add("pin to " + revision.map(RevisionId::toString).orElse("current revision")); + else + revision.ifPresent(revision -> changes.add("revision change to " + revision)); changes.setEmptyValue("no change"); return changes.toString(); } public static Change of(RevisionId revision) { - return new Change(Optional.empty(), Optional.of(revision), false); + return new Change(Optional.empty(), Optional.of(revision), false, false); } public static Change of(Version platformChange) { - return new Change(Optional.of(platformChange), Optional.empty(), false); + return new Change(Optional.of(platformChange), Optional.empty(), false, false); } /** Returns whether this change carries a revision downgrade relative to the given revision. */ diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/application/InstanceList.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/application/InstanceList.java index c1bf083b26c..b94779994e4 100644 --- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/application/InstanceList.java +++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/application/InstanceList.java @@ -125,7 +125,7 @@ public class InstanceList extends AbstractFilteringList<ApplicationId, InstanceL /** Returns the subset of instances which are not pinned to a certain Vespa version. */ public InstanceList unpinned() { - return matching(id -> ! instance(id).change().isPinned()); + return matching(id -> ! instance(id).change().isPlatformPinned()); } /** Returns the subset of instances which are currently failing a job. */ diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/deployment/DeploymentStatus.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/deployment/DeploymentStatus.java index 00da34fe2e4..0f1bbfeb25e 100644 --- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/deployment/DeploymentStatus.java +++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/deployment/DeploymentStatus.java @@ -229,7 +229,7 @@ public class DeploymentStatus { .anyMatch(deployment -> ! compatibleWithCompileVersion.test(deployment.version()))) { for (Version platform : targetsForPolicy(versionStatus, systemVersion, application.deploymentSpec().requireInstance(instance).upgradePolicy())) if (compatibleWithCompileVersion.test(platform)) - return change.withoutPin().with(platform); + return change.withoutPlatformPin().with(platform); } return change; } @@ -265,7 +265,7 @@ public class DeploymentStatus { for (InstanceName instance : application.deploymentSpec().instanceNames()) { Change outstanding = outstandingChange(instance); if (outstanding.hasTargets()) - outstandingChanges.put(instance, outstanding.onTopOf(application.require(instance).change())); + outstandingChanges.put(instance, outstanding.onTopOf(application.require(instance).change().withoutRevisionPin())); } var testJobs = jobsToRun(outstandingChanges, true).entrySet().stream() .filter(entry -> ! entry.getKey().type().isProduction()); @@ -596,7 +596,8 @@ public class DeploymentStatus { /** Changes to deploy with the given job, possibly split in two steps. */ private List<Change> changes(JobId job, StepStatus step, Change change) { - if (change.platform().isEmpty() || change.revision().isEmpty() || change.isPinned()) + if ( change.platform().isEmpty() || change.revision().isEmpty() + || change.isPlatformPinned() || change.isRevisionPinned()) return List.of(change); if ( step.completedAt(change.withoutApplication(), Optional.of(job)).isPresent() @@ -1090,14 +1091,14 @@ public class DeploymentStatus { /** Complete if deployment is on pinned version, and last successful deployment, or if given versions is strictly a downgrade, and this isn't forced by a pin. */ @Override Optional<Instant> completedAt(Change change, Optional<JobId> dependent) { - if ( change.isPinned() + if ( change.isPlatformPinned() && change.platform().isPresent() && ! existingDeployment.map(Deployment::version).equals(change.platform())) return Optional.empty(); if ( change.revision().isPresent() - && ! existingDeployment.map(Deployment::revision).equals(change.revision()) - && dependent.equals(job())) // Job should (re-)run in this case, but other dependents need not wait. + && change.isRevisionPinned() + && ! existingDeployment.map(Deployment::revision).equals(change.revision())) return Optional.empty(); Change fullChange = status.application().require(job.id().application().instance()).change(); diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/deployment/DeploymentTrigger.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/deployment/DeploymentTrigger.java index 00a0e22f87d..4e699f2c28f 100644 --- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/deployment/DeploymentTrigger.java +++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/deployment/DeploymentTrigger.java @@ -41,7 +41,6 @@ import java.util.logging.Logger; import java.util.stream.Collectors; import static java.util.Comparator.comparing; -import static java.util.Comparator.comparingDouble; import static java.util.stream.Collectors.groupingBy; import static java.util.stream.Collectors.toMap; @@ -331,15 +330,14 @@ public class DeploymentTrigger { /** Cancels the indicated part of the given application's change. */ public void cancelChange(ApplicationId instanceId, ChangesToCancel cancellation) { applications().lockApplicationOrThrow(TenantAndApplicationId.from(instanceId), application -> { - Change change; - switch (cancellation) { - case ALL: change = Change.empty(); break; - case VERSIONS: change = Change.empty().withPin(); break; - case PLATFORM: change = application.get().require(instanceId.instance()).change().withoutPlatform(); break; - case APPLICATION: change = application.get().require(instanceId.instance()).change().withoutApplication(); break; - case PIN: change = application.get().require(instanceId.instance()).change().withoutPin(); break; - default: throw new IllegalArgumentException("Unknown cancellation choice '" + cancellation + "'!"); - } + Change change = switch (cancellation) { + case ALL -> Change.empty(); + case PLATFORM -> application.get().require(instanceId.instance()).change().withoutPlatform(); + case APPLICATION -> application.get().require(instanceId.instance()).change().withoutApplication(); + case PIN -> application.get().require(instanceId.instance()).change().withoutPlatformPin(); + case PLATFORM_PIN -> application.get().require(instanceId.instance()).change().withoutPlatformPin(); + case APPLICATION_PIN -> application.get().require(instanceId.instance()).change().withoutRevisionPin(); + }; applications().store(application.with(instanceId.instance(), instance -> withRemainingChange(instance, change, @@ -348,7 +346,7 @@ public class DeploymentTrigger { }); } - public enum ChangesToCancel { ALL, PLATFORM, APPLICATION, VERSIONS, PIN } + public enum ChangesToCancel { ALL, PLATFORM, APPLICATION, PIN, PLATFORM_PIN, APPLICATION_PIN } // ---------- Conveniences ---------- diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/deployment/InternalStepRunner.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/deployment/InternalStepRunner.java index 7b1a1e879d6..52ddcfd5171 100644 --- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/deployment/InternalStepRunner.java +++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/deployment/InternalStepRunner.java @@ -437,7 +437,7 @@ public class InternalStepRunner implements StepRunner { Version targetPlatform = controller.jobController().run(id).versions().targetPlatform(); Version systemVersion = controller.readSystemVersion(); boolean incompatible = controller.applications().versionCompatibility(id.application()).refuse(targetPlatform, systemVersion); - return incompatible || application(id.application()).change().isPinned() ? targetPlatform : systemVersion; + return incompatible || application(id.application()).change().isPlatformPinned() ? targetPlatform : systemVersion; } private Optional<RunStatus> installTester(RunId id, DualLogger logger) { diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/deployment/JobController.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/deployment/JobController.java index 10e4052f067..318a6ffe820 100644 --- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/deployment/JobController.java +++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/deployment/JobController.java @@ -111,6 +111,7 @@ import static java.util.logging.Level.WARNING; public class JobController { public static final Duration maxHistoryAge = Duration.ofDays(60); + public static final Duration obsoletePackageExpiry = Duration.ofDays(7); private static final Logger log = Logger.getLogger(JobController.class.getName()); @@ -165,8 +166,8 @@ public class JobController { return Optional.empty(); return active(id).isPresent() - ? Optional.of(logs.readActive(id.application(), id.type(), after)) - : logs.readFinished(id, after); + ? Optional.of(logs.readActive(id.application(), id.type(), after)) + : logs.readFinished(id, after); } } @@ -284,10 +285,10 @@ public class JobController { private Optional<InputStream> getVespaLogsFromLogserver(Run run, long fromMillis, boolean tester) { return deploymentCompletedAt(run, tester).map(at -> - controller.serviceRegistry().configServer().getLogs(new DeploymentId(tester ? run.id().tester().id() : run.id().application(), - run.id().type().zone()), - Map.of("from", Long.toString(Math.max(fromMillis, at.toEpochMilli())), - "to", Long.toString(run.end().orElse(controller.clock().instant()).toEpochMilli())))); + controller.serviceRegistry().configServer().getLogs(new DeploymentId(tester ? run.id().tester().id() : run.id().application(), + run.id().type().zone()), + Map.of("from", Long.toString(Math.max(fromMillis, at.toEpochMilli())), + "to", Long.toString(run.end().orElse(controller.clock().instant()).toEpochMilli())))); } /** Fetches any new test log entries, and records the id of the last of these, for continuation. */ @@ -509,14 +510,14 @@ public class JobController { long successes = runs.values().stream().filter(Run::hasSucceeded).count(); var oldEntries = runs.entrySet().iterator(); for (var old = oldEntries.next(); - old.getKey().number() <= last - historyLength + old.getKey().number() <= last - historyLength || old.getValue().start().isBefore(controller.clock().instant().minus(maxHistoryAge)); old = oldEntries.next()) { // Make sure we keep the last success and the first failing if ( successes == 1 - && old.getValue().hasSucceeded() - && ! old.getValue().start().isBefore(controller.clock().instant().minus(maxHistoryAge))) { + && old.getValue().hasSucceeded() + && ! old.getValue().start().isBefore(controller.clock().instant().minus(maxHistoryAge))) { oldEntries.next(); continue; } @@ -624,7 +625,7 @@ public class JobController { }); } - private LockedApplication withPrunedPackages(LockedApplication application, RevisionId latest){ + private LockedApplication withPrunedPackages(LockedApplication application, RevisionId latest) { TenantAndApplicationId id = application.get().id(); Application wrapped = application.get(); RevisionId oldestDeployed = application.get().oldestDeployedRevision() @@ -632,11 +633,28 @@ public class JobController { .flatMap(instance -> instance.change().revision().stream()) .min(naturalOrder())) .orElse(latest); - controller.applications().applicationStore().prune(id.tenant(), id.application(), oldestDeployed); + RevisionId oldestToKeep = null; + Instant now = controller.clock().instant(); + for (ApplicationVersion version : application.get().revisions().withPackage()) { + if (version.id().compareTo(oldestDeployed) < 0) { + if (version.obsoleteAt().isEmpty()) { + application = application.withRevisions(revisions -> revisions.with(version.obsoleteAt(now))); + if (oldestToKeep == null) + oldestToKeep = version.id(); + } + else { + if (oldestToKeep == null && !version.obsoleteAt().get().isBefore(now.minus(obsoletePackageExpiry))) + oldestToKeep = version.id(); + } + } + } - for (ApplicationVersion version : application.get().revisions().withPackage()) - if (version.id().compareTo(oldestDeployed) < 0) - application = application.withRevisions(revisions -> revisions.with(version.withoutPackage())); + if (oldestToKeep != null) { + controller.applications().applicationStore().prune(id.tenant(), id.application(), oldestToKeep); + for (ApplicationVersion version : application.get().revisions().withPackage()) + if (version.id().compareTo(oldestToKeep) < 0) + application = application.withRevisions(revisions -> revisions.with(version.withoutPackage())); + } return application; } @@ -703,8 +721,8 @@ public class JobController { VersionStatus versionStatus = controller.readVersionStatus(); if ( ! controller.system().isCd() - && platform.isPresent() - && versionStatus.deployableVersions().stream().map(VespaVersion::versionNumber).noneMatch(platform.get()::equals)) + && platform.isPresent() + && versionStatus.deployableVersions().stream().map(VespaVersion::versionNumber).noneMatch(platform.get()::equals)) throw new IllegalArgumentException("platform version " + platform.get() + " is not present in this system"); controller.applications().lockApplicationOrThrow(TenantAndApplicationId.from(id), application -> { @@ -731,8 +749,8 @@ public class JobController { controller.applications().lockApplicationOrThrow(TenantAndApplicationId.from(id), application -> { Version targetPlatform = platform.orElseGet(() -> findTargetPlatform(applicationPackage, deploymentId, application.get().get(id.instance()), versionStatus)); if ( ! allowOutdatedPlatform - && ! controller.readVersionStatus().isOnCurrentMajor(targetPlatform) - && runs(id, type).values().stream().noneMatch(run -> run.versions().targetPlatform().getMajor() == targetPlatform.getMajor())) + && ! controller.readVersionStatus().isOnCurrentMajor(targetPlatform) + && runs(id, type).values().stream().noneMatch(run -> run.versions().targetPlatform().getMajor() == targetPlatform.getMajor())) throw new IllegalArgumentException("platform version " + targetPlatform + " is not on a current major version in this system"); controller.applications().applicationStore().putDev(deploymentId, version.id(), applicationPackage.zippedContent(), diff); @@ -872,7 +890,7 @@ public class JobController { /** Locks all runs and modifies the list of historic runs for the given application and job type. */ private void locked(ApplicationId id, JobType type, Consumer<SortedMap<RunId, Run>> modifications) { - try (Mutex __ = curator.lock(id, type)) { + try (Mutex __ = curator.lock(id, type)) { SortedMap<RunId, Run> runs = new TreeMap<>(curator.readHistoricRuns(id, type)); modifications.accept(runs); curator.writeHistoricRuns(id, type, runs.values()); diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/deployment/RevisionHistory.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/deployment/RevisionHistory.java index bbab9487ea2..272417ba0ac 100644 --- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/deployment/RevisionHistory.java +++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/deployment/RevisionHistory.java @@ -93,7 +93,7 @@ public class RevisionHistory { // Fallback for when an application version isn't known for the given key. private static ApplicationVersion revisionOf(RevisionId id) { - return new ApplicationVersion(id, Optional.empty(), Optional.empty(), Optional.empty(), Optional.empty(), Optional.empty(), Optional.empty(), Optional.empty(), Optional.empty(), false, false, Optional.empty(), 0); + return new ApplicationVersion(id, Optional.empty(), Optional.empty(), Optional.empty(), Optional.empty(), Optional.empty(), Optional.empty(), Optional.empty(), Optional.empty(), Optional.empty(), false, false, Optional.empty(), 0); } /** Returns the production {@link ApplicationVersion} with this revision ID. */ diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/deployment/Versions.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/deployment/Versions.java index e7371561636..f752e396c09 100644 --- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/deployment/Versions.java +++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/deployment/Versions.java @@ -126,7 +126,7 @@ public class Versions { private static Version targetPlatform(Application application, Change change, Optional<Version> existing, Supplier<Version> defaultVersion) { - if (change.isPinned() && change.platform().isPresent()) + if (change.isPlatformPinned() && change.platform().isPresent()) return change.platform().get(); return max(change.platform(), existing) @@ -135,6 +135,9 @@ public class Versions { private static RevisionId targetRevision(Application application, Change change, Optional<RevisionId> existing) { + if (change.isRevisionPinned() && change.revision().isPresent()) + return change.revision().get(); + return change.revision() .or(() -> existing) .orElseGet(() -> defaultRevision(application)); diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/maintenance/VcmrMaintainer.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/maintenance/VcmrMaintainer.java index 9271f870390..d4c4b4efda7 100644 --- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/maintenance/VcmrMaintainer.java +++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/maintenance/VcmrMaintainer.java @@ -139,7 +139,10 @@ public class VcmrMaintainer extends ControllerMaintainer { return Stream.empty(); } var spareCapacity = hasSpareCapacity(zone, nodes); - return nodes.stream().map(node -> nextAction(zone, node, changeRequest, spareCapacity)); + var impactedProxyCount = nodes.stream() + .filter(node -> node.type() == NodeType.proxy) + .count(); + return nodes.stream().map(node -> nextAction(zone, node, changeRequest, spareCapacity, impactedProxyCount)); }).toList(); } @@ -162,7 +165,7 @@ public class VcmrMaintainer extends ControllerMaintainer { .findFirst(); } - private HostAction nextAction(ZoneId zoneId, Node node, VespaChangeRequest changeRequest, boolean spareCapacity) { + private HostAction nextAction(ZoneId zoneId, Node node, VespaChangeRequest changeRequest, boolean spareCapacity, long impactedProxyCount) { var hostAction = getPreviousAction(node, changeRequest) .orElse(new HostAction(node.hostname().value(), State.NONE, Instant.now())); @@ -176,7 +179,8 @@ public class VcmrMaintainer extends ControllerMaintainer { if (isLowImpact(changeRequest)) return hostAction; - addReport(zoneId, changeRequest, node); + if (shouldAddReport(node, changeRequest.getChangeRequestSource().getId(), hostAction)) + addReport(zoneId, changeRequest, node); if (isOutOfSync(node, hostAction)) return hostAction.withState(State.OUT_OF_SYNC); @@ -187,7 +191,13 @@ public class VcmrMaintainer extends ControllerMaintainer { return hostAction.withState(State.PENDING_RETIREMENT); } - if (node.type() != NodeType.host || !spareCapacity) { + if (!spareCapacity) { + return hostAction.withState(State.REQUIRES_OPERATOR_ACTION); + } + + if (node.type() != NodeType.host) { + if (node.type() == NodeType.proxy && impactedProxyCount == 1) + return hostAction.withState(State.READY); return hostAction.withState(State.REQUIRES_OPERATOR_ACTION); } @@ -267,6 +277,16 @@ public class VcmrMaintainer extends ControllerMaintainer { && node.state() == Node.State.active; } + private boolean shouldAddReport(Node node, String vcmrId, HostAction previousAction) { + var vcmrReport = VcmrReport.fromReports(node.reports()); + var hasReport = vcmrReport.getVcmrs().stream().map(VcmrReport.Vcmr::id).anyMatch(id -> id.equals(vcmrId)); + // Don't add report if none exists and this is not initial assessment + // Presumably removed manually by operator. + if (!hasReport && previousAction.getState() != State.NONE) + return false; + return true; + } + // Determines if node state is unexpected based on previous action taken private boolean isOutOfSync(Node node, HostAction action) { return action.getState() == State.RETIRED && node.state() != Node.State.parked || @@ -343,8 +363,7 @@ public class VcmrMaintainer extends ControllerMaintainer { private void addReport(ZoneId zoneId, VespaChangeRequest changeRequest, Node node) { var report = VcmrReport.fromReports(node.reports()); - var source = changeRequest.getChangeRequestSource(); - if (report.addVcmr(source.getId(), source.getPlannedStartTime(), source.getPlannedEndTime())) { + if (report.addVcmr(changeRequest.getChangeRequestSource())) { updateReport(zoneId, node, report); } } diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/persistence/ApplicationSerializer.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/persistence/ApplicationSerializer.java index ee12c9957b1..e5006ab9785 100644 --- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/persistence/ApplicationSerializer.java +++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/persistence/ApplicationSerializer.java @@ -82,7 +82,8 @@ public class ApplicationSerializer { private static final String versionsField = "versions"; private static final String prodVersionsField = "prodVersions"; private static final String devVersionsField = "devVersions"; - private static final String pinnedField = "pinned"; + private static final String platformPinnedField = "pinned"; + private static final String revisionPinnedField = "revisionPinned"; private static final String deploymentIssueField = "deploymentIssueId"; private static final String ownershipIssueIdField = "ownershipIssueId"; private static final String ownerField = "confirmedOwner"; @@ -118,6 +119,7 @@ public class ApplicationSerializer { private static final String riskField = "risk"; private static final String authorEmailField = "authorEmailField"; private static final String deployedDirectlyField = "deployedDirectly"; + private static final String obsoleteAtField = "obsoleteAt"; private static final String hasPackageField = "hasPackage"; private static final String shouldSkipField = "shouldSkip"; private static final String compileVersionField = "compileVersion"; @@ -265,6 +267,7 @@ public class ApplicationSerializer { applicationVersion.sourceUrl().ifPresent(url -> object.setString(sourceUrlField, url)); applicationVersion.commit().ifPresent(commit -> object.setString(commitField, commit)); object.setBool(deployedDirectlyField, applicationVersion.isDeployedDirectly()); + applicationVersion.obsoleteAt().ifPresent(at -> object.setLong(obsoleteAtField, at.toEpochMilli())); object.setBool(hasPackageField, applicationVersion.hasPackage()); object.setBool(shouldSkipField, applicationVersion.shouldSkip()); applicationVersion.description().ifPresent(description -> object.setString(descriptionField, description)); @@ -295,8 +298,10 @@ public class ApplicationSerializer { object.setString(versionField, deploying.platform().get().toString()); if (deploying.revision().isPresent()) toSlime(deploying.revision().get(), object); - if (deploying.isPinned()) - object.setBool(pinnedField, true); + if (deploying.isPlatformPinned()) + object.setBool(platformPinnedField, true); + if (deploying.isRevisionPinned()) + object.setBool(revisionPinnedField, true); } private void toSlime(RotationStatus status, Cursor array) { @@ -487,6 +492,7 @@ public class ApplicationSerializer { Optional<Instant> buildTime = SlimeUtils.optionalInstant(object.field(buildTimeField)); Optional<String> sourceUrl = SlimeUtils.optionalString(object.field(sourceUrlField)); Optional<String> commit = SlimeUtils.optionalString(object.field(commitField)); + Optional<Instant> obsoleteAt = SlimeUtils.optionalInstant(object.field(obsoleteAtField)); boolean hasPackage = object.field(hasPackageField).asBool(); boolean shouldSkip = object.field(shouldSkipField).asBool(); Optional<String> description = SlimeUtils.optionalString(object.field(descriptionField)); @@ -494,7 +500,7 @@ public class ApplicationSerializer { Optional<String> bundleHash = SlimeUtils.optionalString(object.field(bundleHashField)); return new ApplicationVersion(id, sourceRevision, authorEmail, compileVersion, allowedMajor, buildTime, - sourceUrl, commit, bundleHash, hasPackage, shouldSkip, description, risk); + sourceUrl, commit, bundleHash, obsoleteAt, hasPackage, shouldSkip, description, risk); } private Optional<SourceRevision> sourceRevisionFromSlime(Inspector object) { @@ -520,8 +526,10 @@ public class ApplicationSerializer { change = Change.of(Version.fromString(versionFieldValue.asString())); if (object.field(applicationBuildNumberField).valid()) change = change.with(revisionFromSlime(object, null)); - if (object.field(pinnedField).asBool()) - change = change.withPin(); + if (object.field(platformPinnedField).asBool()) + change = change.withPlatformPin(); + if (object.field(revisionPinnedField).asBool()) + change = change.withRevisionPin(); return change; } diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/restapi/application/ApplicationApiHandler.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/restapi/application/ApplicationApiHandler.java index 81988753621..ded27ee1060 100644 --- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/restapi/application/ApplicationApiHandler.java +++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/restapi/application/ApplicationApiHandler.java @@ -260,11 +260,9 @@ public class ApplicationApiHandler extends AuditLoggingRequestHandler { if (path.matches("/application/v4/tenant/{tenant}/application/{application}/package")) return applicationPackage(path.get("tenant"), path.get("application"), request); if (path.matches("/application/v4/tenant/{tenant}/application/{application}/diff/{number}")) return applicationPackageDiff(path.get("tenant"), path.get("application"), path.get("number")); if (path.matches("/application/v4/tenant/{tenant}/application/{application}/deploying")) return deploying(path.get("tenant"), path.get("application"), "default", request); - if (path.matches("/application/v4/tenant/{tenant}/application/{application}/deploying/pin")) return deploying(path.get("tenant"), path.get("application"), "default", request); if (path.matches("/application/v4/tenant/{tenant}/application/{application}/instance")) return applications(path.get("tenant"), Optional.of(path.get("application")), request); if (path.matches("/application/v4/tenant/{tenant}/application/{application}/instance/{instance}")) return instance(path.get("tenant"), path.get("application"), path.get("instance"), request); if (path.matches("/application/v4/tenant/{tenant}/application/{application}/instance/{instance}/deploying")) return deploying(path.get("tenant"), path.get("application"), path.get("instance"), request); - if (path.matches("/application/v4/tenant/{tenant}/application/{application}/instance/{instance}/deploying/pin")) return deploying(path.get("tenant"), path.get("application"), path.get("instance"), request); if (path.matches("/application/v4/tenant/{tenant}/application/{application}/instance/{instance}/job")) return JobControllerApiHandlerHelper.jobTypeResponse(controller, appIdFromPath(path), request.getUri()); if (path.matches("/application/v4/tenant/{tenant}/application/{application}/instance/{instance}/job/{jobtype}")) return JobControllerApiHandlerHelper.runResponse(controller.applications().requireApplication(TenantAndApplicationId.from(path.get("tenant"), path.get("application"))), controller.jobController().runs(appIdFromPath(path), jobTypeFromPath(path)).descendingMap(), Optional.ofNullable(request.getProperty("limit")), request.getUri()); // (((＼（✘෴✘）／))) if (path.matches("/application/v4/tenant/{tenant}/application/{application}/instance/{instance}/job/{jobtype}/package")) return devApplicationPackage(appIdFromPath(path), jobTypeFromPath(path)); @@ -327,14 +325,18 @@ public class ApplicationApiHandler extends AuditLoggingRequestHandler { if (path.matches("/application/v4/tenant/{tenant}/application/{application}")) return createApplication(path.get("tenant"), path.get("application"), request); if (path.matches("/application/v4/tenant/{tenant}/application/{application}/deploying/platform")) return deployPlatform(path.get("tenant"), path.get("application"), "default", false, request); if (path.matches("/application/v4/tenant/{tenant}/application/{application}/deploying/pin")) return deployPlatform(path.get("tenant"), path.get("application"), "default", true, request); - if (path.matches("/application/v4/tenant/{tenant}/application/{application}/deploying/application")) return deployApplication(path.get("tenant"), path.get("application"), "default", request); + if (path.matches("/application/v4/tenant/{tenant}/application/{application}/deploying/platform-pin")) return deployPlatform(path.get("tenant"), path.get("application"), "default", true, request); + if (path.matches("/application/v4/tenant/{tenant}/application/{application}/deploying/application-pin")) return deployApplication(path.get("tenant"), path.get("application"), "default", true, request); + if (path.matches("/application/v4/tenant/{tenant}/application/{application}/deploying/application")) return deployApplication(path.get("tenant"), path.get("application"), "default", false, request); if (path.matches("/application/v4/tenant/{tenant}/application/{application}/key")) return addDeployKey(path.get("tenant"), path.get("application"), request); if (path.matches("/application/v4/tenant/{tenant}/application/{application}/submit")) return submit(path.get("tenant"), path.get("application"), request); if (path.matches("/application/v4/tenant/{tenant}/application/{application}/instance/{instance}")) return createInstance(path.get("tenant"), path.get("application"), path.get("instance"), request); if (path.matches("/application/v4/tenant/{tenant}/application/{application}/instance/{instance}/deploy/{jobtype}")) return jobDeploy(appIdFromPath(path), jobTypeFromPath(path), request); if (path.matches("/application/v4/tenant/{tenant}/application/{application}/instance/{instance}/deploying/platform")) return deployPlatform(path.get("tenant"), path.get("application"), path.get("instance"), false, request); if (path.matches("/application/v4/tenant/{tenant}/application/{application}/instance/{instance}/deploying/pin")) return deployPlatform(path.get("tenant"), path.get("application"), path.get("instance"), true, request); - if (path.matches("/application/v4/tenant/{tenant}/application/{application}/instance/{instance}/deploying/application")) return deployApplication(path.get("tenant"), path.get("application"), path.get("instance"), request); + if (path.matches("/application/v4/tenant/{tenant}/application/{application}/instance/{instance}/deploying/platform-pin")) return deployPlatform(path.get("tenant"), path.get("application"), path.get("instance"), true, request); + if (path.matches("/application/v4/tenant/{tenant}/application/{application}/instance/{instance}/deploying/application-pin")) return deployApplication(path.get("tenant"), path.get("application"), path.get("instance"), true, request); + if (path.matches("/application/v4/tenant/{tenant}/application/{application}/instance/{instance}/deploying/application")) return deployApplication(path.get("tenant"), path.get("application"), path.get("instance"), false, request); if (path.matches("/application/v4/tenant/{tenant}/application/{application}/instance/{instance}/submit")) return submit(path.get("tenant"), path.get("application"), request); if (path.matches("/application/v4/tenant/{tenant}/application/{application}/instance/{instance}/job/{jobtype}")) return trigger(appIdFromPath(path), jobTypeFromPath(path), request); if (path.matches("/application/v4/tenant/{tenant}/application/{application}/instance/{instance}/job/{jobtype}/pause")) return pause(appIdFromPath(path), jobTypeFromPath(path)); @@ -2059,7 +2061,9 @@ public class ApplicationApiHandler extends AuditLoggingRequestHandler { if ( ! instance.change().isEmpty()) { instance.change().platform().ifPresent(version -> root.setString("platform", version.toString())); instance.change().revision().ifPresent(revision -> root.setString("application", revision.toString())); - root.setBool("pinned", instance.change().isPinned()); + root.setBool("pinned", instance.change().isPlatformPinned()); + root.setBool("platform-pinned", instance.change().isPlatformPinned()); + root.setBool("application-pinned", instance.change().isRevisionPinned()); } return new SlimeJsonResponse(slime); } @@ -2172,7 +2176,7 @@ public class ApplicationApiHandler extends AuditLoggingRequestHandler { .collect(joining(", "))); Change change = Change.of(version); if (pin) - change = change.withPin(); + change = change.withPlatformPin(); controller.applications().deploymentTrigger().forceChange(id, change, isOperator(request)); response.append("Triggered ").append(change).append(" for ").append(id); @@ -2181,7 +2185,7 @@ public class ApplicationApiHandler extends AuditLoggingRequestHandler { } /** Trigger deployment to the last known application package for the given application. */ - private HttpResponse deployApplication(String tenantName, String applicationName, String instanceName, HttpRequest request) { + private HttpResponse deployApplication(String tenantName, String applicationName, String instanceName, boolean pin, HttpRequest request) { ApplicationId id = ApplicationId.from(tenantName, applicationName, instanceName); Inspector buildField = toSlime(request.getData()).get().field("build"); long build = buildField.valid() ? buildField.asLong() : -1; @@ -2191,6 +2195,8 @@ public class ApplicationApiHandler extends AuditLoggingRequestHandler { RevisionId revision = build == -1 ? application.get().revisions().last().get().id() : getRevision(application.get(), build); Change change = Change.of(revision); + if (pin) + change = change.withRevisionPin(); controller.applications().deploymentTrigger().forceChange(id, change, isOperator(request)); response.append("Triggered ").append(change).append(" for ").append(id); }); @@ -2231,7 +2237,7 @@ public class ApplicationApiHandler extends AuditLoggingRequestHandler { return; } - ChangesToCancel cancel = ChangesToCancel.valueOf(choice.toUpperCase()); + ChangesToCancel cancel = ChangesToCancel.valueOf(choice.replaceAll("-", "_").toUpperCase()); controller.applications().deploymentTrigger().cancelChange(id, cancel); response.append("Changed deployment from '").append(change).append("' to '").append(controller.applications().requireInstance(id).change()).append("' for ").append(id); }); diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/restapi/application/JobControllerApiHandlerHelper.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/restapi/application/JobControllerApiHandlerHelper.java index 804ae7b7805..9ff8c7df18b 100644 --- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/restapi/application/JobControllerApiHandlerHelper.java +++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/restapi/application/JobControllerApiHandlerHelper.java @@ -312,7 +312,9 @@ class JobControllerApiHandlerHelper { if ( ! change.isEmpty()) { change.platform().ifPresent(version -> deployingObject.setString("platform", version.toFullString())); change.revision().ifPresent(revision -> toSlime(deployingObject.setObject("application"), application.revisions().get(revision))); - if (change.isPinned()) deployingObject.setBool("pinned", true); + if (change.isPlatformPinned()) deployingObject.setBool("pinned", true); + if (change.isPlatformPinned()) deployingObject.setBool("platformPinned", true); + if (change.isRevisionPinned()) deployingObject.setBool("revisionPinned", true); } Cursor latestVersionsObject = stepObject.setObject("latestVersions"); diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/restapi/deployment/DeploymentApiHandler.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/restapi/deployment/DeploymentApiHandler.java index 069ee58e9c5..6e5635e8c8c 100644 --- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/restapi/deployment/DeploymentApiHandler.java +++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/restapi/deployment/DeploymentApiHandler.java @@ -171,7 +171,9 @@ public class DeploymentApiHandler extends ThreadedHttpRequestHandler { instanceObject.setString("application", instance.application().value()); instanceObject.setString("instance", instance.instance().value()); instanceObject.setBool("upgrading", status.application().require(instance.instance()).change().platform().equals(Optional.of(statistics.version()))); - instanceObject.setBool("pinned", status.application().require(instance.instance()).change().isPinned()); + instanceObject.setBool("pinned", status.application().require(instance.instance()).change().isPlatformPinned()); + instanceObject.setBool("platformPinned", status.application().require(instance.instance()).change().isPlatformPinned()); + instanceObject.setBool("revisionPinned", status.application().require(instance.instance()).change().isRevisionPinned()); DeploymentStatus.StepStatus stepStatus = status.instanceSteps().get(instance.instance()); if (stepStatus != null) { // Instance may not have any steps, i.e. an empty deployment spec has been submitted Readiness platformReadiness = stepStatus.blockedUntil(Change.of(statistics.version())); diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/ControllerTest.java b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/ControllerTest.java index a9a6fe602b6..04c8c46e1ef 100644 --- a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/ControllerTest.java +++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/ControllerTest.java @@ -41,6 +41,7 @@ import com.yahoo.vespa.hosted.controller.application.pkg.ApplicationPackage; import com.yahoo.vespa.hosted.controller.deployment.ApplicationPackageBuilder; import com.yahoo.vespa.hosted.controller.deployment.DeploymentContext; import com.yahoo.vespa.hosted.controller.deployment.DeploymentTester; +import com.yahoo.vespa.hosted.controller.deployment.JobController; import com.yahoo.vespa.hosted.controller.deployment.Submission; import com.yahoo.vespa.hosted.controller.integration.ZoneApiMock; import com.yahoo.vespa.hosted.controller.notification.Notification; @@ -106,9 +107,11 @@ public class ControllerTest { Version version1 = tester.configServer().initialVersion(); var context = tester.newDeploymentContext(); context.submit(applicationPackage); - assertEquals(ApplicationVersion.from(RevisionId.forProduction(1), DeploymentContext.defaultSourceRevision, "a@b", new Version("6.1"), Instant.ofEpochSecond(1)), - context.application().revisions().get(context.instance().change().revision().get()), - "Application version is known from completion of initial job"); + RevisionId id = RevisionId.forProduction(1); + Version compileVersion = new Version("6.1"); + assertEquals(new ApplicationVersion(id, Optional.of(DeploymentContext.defaultSourceRevision), Optional.of("a@b"), Optional.of(compileVersion), Optional.empty(), Optional.of(Instant.ofEpochSecond(1)), Optional.empty(), Optional.empty(), Optional.empty(), Optional.empty(), true, false, Optional.empty(), 0), + context.application().revisions().get(context.instance().change().revision().get()), + "Application version is known from completion of initial job"); context.runJob(systemTest); context.runJob(stagingTest); @@ -220,6 +223,59 @@ public class ControllerTest { } @Test + void testPackagePruning() { + DeploymentContext app = tester.newDeploymentContext().submit().deploy(); + RevisionId revision1 = app.lastSubmission().get(); + assertTrue(tester.controllerTester().serviceRegistry().applicationStore() + .hasBuild(app.instanceId().tenant(), app.instanceId().application(), revision1.number())); + + app.submit().deploy(); + RevisionId revision2 = app.lastSubmission().get(); + assertTrue(tester.controllerTester().serviceRegistry().applicationStore() + .hasBuild(app.instanceId().tenant(), app.instanceId().application(), revision1.number())); + assertTrue(tester.controllerTester().serviceRegistry().applicationStore() + .hasBuild(app.instanceId().tenant(), app.instanceId().application(), revision2.number())); + + // Revision 1 is marked as obsolete now + app.submit().deploy(); + RevisionId revision3 = app.lastSubmission().get(); + assertTrue(tester.controllerTester().serviceRegistry().applicationStore() + .hasBuild(app.instanceId().tenant(), app.instanceId().application(), revision1.number())); + assertTrue(tester.controllerTester().serviceRegistry().applicationStore() + .hasBuild(app.instanceId().tenant(), app.instanceId().application(), revision2.number())); + assertTrue(tester.controllerTester().serviceRegistry().applicationStore() + .hasBuild(app.instanceId().tenant(), app.instanceId().application(), revision3.number())); + + // Time advances, and revision 2 is marked as obsolete now + tester.clock().advance(JobController.obsoletePackageExpiry); + app.submit().deploy(); + RevisionId revision4 = app.lastSubmission().get(); + assertTrue(tester.controllerTester().serviceRegistry().applicationStore() + .hasBuild(app.instanceId().tenant(), app.instanceId().application(), revision1.number())); + assertTrue(tester.controllerTester().serviceRegistry().applicationStore() + .hasBuild(app.instanceId().tenant(), app.instanceId().application(), revision2.number())); + assertTrue(tester.controllerTester().serviceRegistry().applicationStore() + .hasBuild(app.instanceId().tenant(), app.instanceId().application(), revision3.number())); + assertTrue(tester.controllerTester().serviceRegistry().applicationStore() + .hasBuild(app.instanceId().tenant(), app.instanceId().application(), revision4.number())); + + // Time advances, and revision is now old enough to be pruned + tester.clock().advance(Duration.ofMillis(1)); + app.submit().deploy(); + RevisionId revision5 = app.lastSubmission().get(); + assertFalse(tester.controllerTester().serviceRegistry().applicationStore() + .hasBuild(app.instanceId().tenant(), app.instanceId().application(), revision1.number())); + assertTrue(tester.controllerTester().serviceRegistry().applicationStore() + .hasBuild(app.instanceId().tenant(), app.instanceId().application(), revision2.number())); + assertTrue(tester.controllerTester().serviceRegistry().applicationStore() + .hasBuild(app.instanceId().tenant(), app.instanceId().application(), revision3.number())); + assertTrue(tester.controllerTester().serviceRegistry().applicationStore() + .hasBuild(app.instanceId().tenant(), app.instanceId().application(), revision4.number())); + assertTrue(tester.controllerTester().serviceRegistry().applicationStore() + .hasBuild(app.instanceId().tenant(), app.instanceId().application(), revision5.number())); + } + + @Test void testGlobalRotationStatus() { var context = tester.newDeploymentContext(); var zone1 = ZoneId.from("prod", "us-west-1"); diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/deployment/DeploymentTriggerTest.java b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/deployment/DeploymentTriggerTest.java index afb92d84f3b..6e5c2458c92 100644 --- a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/deployment/DeploymentTriggerTest.java +++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/deployment/DeploymentTriggerTest.java @@ -653,15 +653,21 @@ public class DeploymentTriggerTest { assertEquals(appVersion1, latestDeployed(app.instance())); // Downgrading application version. - tester.deploymentTrigger().forceChange(app.instanceId(), Change.of(appVersion0)); - assertEquals(Change.of(appVersion0), app.instance().change()); + tester.deploymentTrigger().forceChange(app.instanceId(), Change.of(appVersion0).withRevisionPin()); + assertEquals(Change.of(appVersion0).withRevisionPin(), app.instance().change()); app.runJob(stagingTest) - .runJob(productionUsCentral1) - .runJob(productionUsEast3) - .runJob(productionUsWest1); - assertEquals(Change.empty(), app.instance().change()); + .runJob(productionUsCentral1) + .runJob(productionUsEast3) + .runJob(productionUsWest1); + assertEquals(Change.empty().withRevisionPin(), app.instance().change()); assertEquals(appVersion0, app.instance().deployments().get(productionUsEast3.zone()).revision()); assertEquals(appVersion0, latestDeployed(app.instance())); + + tester.outstandingChangeDeployer().run(); + assertEquals(Change.empty().withRevisionPin(), app.instance().change()); + tester.deploymentTrigger().cancelChange(app.instanceId(), ALL); + tester.outstandingChangeDeployer().run(); + assertEquals(Change.of(appVersion1), app.instance().change()); } @Test @@ -1239,13 +1245,13 @@ public class DeploymentTriggerTest { assertEquals(Change.empty(), app.instance().change()); // Application is pinned to previous version, and downgrades to that. Tests are re-run. - tester.deploymentTrigger().forceChange(app.instanceId(), Change.of(version0).withPin()); + tester.deploymentTrigger().forceChange(app.instanceId(), Change.of(version0).withPlatformPin()); app.runJob(stagingTest).runJob(productionUsEast3); tester.clock().advance(Duration.ofMinutes(1)); app.failDeployment(testUsEast3); tester.clock().advance(Duration.ofMinutes(11)); // Job is cooling down after consecutive failures. app.runJob(testUsEast3); - assertEquals(Change.empty().withPin(), app.instance().change()); + assertEquals(Change.empty().withPlatformPin(), app.instance().change()); // A new upgrade is attempted, and production tests wait for redeployment. tester.controllerTester().upgradeSystem(version2); @@ -2234,7 +2240,7 @@ public class DeploymentTriggerTest { .majorVersion(7) .compileVersion(version1) .build()); - tester.deploymentTrigger().forceChange(app.instanceId(), app.instance().change().withPin()); + tester.deploymentTrigger().forceChange(app.instanceId(), app.instance().change().withPlatformPin()); app.deploy(); assertEquals(version1, tester.jobs().last(app.instanceId(), productionUsEast3).get().versions().targetPlatform()); assertEquals(version1, app.application().revisions().get(tester.jobs().last(app.instanceId(), productionUsEast3).get().versions().targetRevision()).compileVersion().get()); @@ -2251,7 +2257,7 @@ public class DeploymentTriggerTest { // The new app enters a platform block window, and is pinned to the old platform; // the new submission overrides both those settings, as the new revision should roll out regardless. tester.atMondayMorning(); - tester.deploymentTrigger().forceChange(newApp.instanceId(), Change.empty().withPin()); + tester.deploymentTrigger().forceChange(newApp.instanceId(), Change.empty().withPlatformPin()); newApp.submit(new ApplicationPackageBuilder().compileVersion(version2) .systemTest() .blockChange(false, true, "mon", "0-23", "UTC") @@ -2280,11 +2286,11 @@ public class DeploymentTriggerTest { tester.upgrader().run(); assertEquals(Change.of(newRevision).with(version1), newApp.instance().change()); - tester.deploymentTrigger().forceChange(newApp.instanceId(), newApp.instance().change().withPin()); + tester.deploymentTrigger().forceChange(newApp.instanceId(), newApp.instance().change().withPlatformPin()); tester.outstandingChangeDeployer().run(); - assertEquals(Change.of(newRevision).with(version1).withPin(), newApp.instance().change()); + assertEquals(Change.of(newRevision).with(version1).withPlatformPin(), newApp.instance().change()); tester.upgrader().run(); - assertEquals(Change.of(newRevision).with(version1).withPin(), newApp.instance().change()); + assertEquals(Change.of(newRevision).with(version1).withPlatformPin(), newApp.instance().change()); newApp.deploy(); assertEquals(version1, tester.jobs().last(newApp.instanceId(), productionUsEast3).get().versions().targetPlatform()); @@ -2381,7 +2387,7 @@ public class DeploymentTriggerTest { .build())) .getMessage()); - tester.deploymentTrigger().forceChange(app.instanceId(), Change.of(oldVersion).with(app.application().revisions().last().get().id()).withPin()); + tester.deploymentTrigger().forceChange(app.instanceId(), Change.of(oldVersion).with(app.application().revisions().last().get().id()).withPlatformPin()); app.deploy(); assertEquals(oldVersion, app.deployment(ZoneId.from("prod", "us-east-3")).version()); diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/maintenance/UpgraderTest.java b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/maintenance/UpgraderTest.java index 11110d6edaa..96c1d7c545d 100644 --- a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/maintenance/UpgraderTest.java +++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/maintenance/UpgraderTest.java @@ -5,7 +5,6 @@ import com.yahoo.component.Version; import com.yahoo.config.provision.ApplicationId; import com.yahoo.config.provision.zone.ZoneId; import com.yahoo.test.ManualClock; -import com.yahoo.vespa.hosted.controller.api.integration.deployment.JobType; import com.yahoo.vespa.hosted.controller.api.integration.deployment.RevisionId; import com.yahoo.vespa.hosted.controller.api.integration.deployment.RunId; import com.yahoo.vespa.hosted.controller.application.Change; @@ -27,7 +26,6 @@ import java.util.HashSet; import java.util.List; import java.util.Map; import java.util.Optional; -import java.util.OptionalInt; import java.util.Set; import java.util.stream.Collectors; @@ -856,10 +854,10 @@ public class UpgraderTest { // Create an application with pinned platform version. var context = tester.newDeploymentContext().submit().deploy(); - tester.deploymentTrigger().forceChange(context.instanceId(), Change.empty().withPin()); + tester.deploymentTrigger().forceChange(context.instanceId(), Change.empty().withPlatformPin()); assertFalse(context.instance().change().hasTargets()); - assertTrue(context.instance().change().isPinned()); + assertTrue(context.instance().change().isPlatformPinned()); assertEquals(3, context.instance().deployments().size()); // Application does not upgrade. @@ -867,21 +865,21 @@ public class UpgraderTest { tester.controllerTester().upgradeSystem(version1); tester.upgrader().maintain(); assertFalse(context.instance().change().hasTargets()); - assertTrue(context.instance().change().isPinned()); + assertTrue(context.instance().change().isPlatformPinned()); // New application package is deployed. context.submit().deploy(); assertFalse(context.instance().change().hasTargets()); - assertTrue(context.instance().change().isPinned()); + assertTrue(context.instance().change().isPlatformPinned()); // Application upgrades to new version when pin is removed. tester.deploymentTrigger().cancelChange(context.instanceId(), PIN); tester.upgrader().maintain(); assertTrue(context.instance().change().hasTargets()); - assertFalse(context.instance().change().isPinned()); + assertFalse(context.instance().change().isPlatformPinned()); // Application is pinned to new version, and upgrade is therefore not cancelled, even though confidence is broken. - tester.deploymentTrigger().forceChange(context.instanceId(), Change.empty().withPin()); + tester.deploymentTrigger().forceChange(context.instanceId(), Change.empty().withPlatformPin()); tester.upgrader().maintain(); tester.triggerJobs(); assertEquals(version1, context.instance().change().platform().get()); @@ -890,7 +888,7 @@ public class UpgraderTest { context.runJob(systemTest).runJob(stagingTest).runJob(productionUsCentral1) .timeOutUpgrade(productionUsWest1); tester.deploymentTrigger().cancelChange(context.instanceId(), ALL); - tester.deploymentTrigger().forceChange(context.instanceId(), Change.of(version0).withPin()); + tester.deploymentTrigger().forceChange(context.instanceId(), Change.of(version0).withPlatformPin()); assertEquals(version0, context.instance().change().platform().get()); // Application downgrades to pinned version. @@ -913,7 +911,7 @@ public class UpgraderTest { // Keep app 1 on current version tester.controller().applications().lockApplicationIfPresent(app1.application().id(), app -> tester.controller().applications().store(app.with(app1.instance().name(), - instance -> instance.withChange(instance.change().withPin())))); + instance -> instance.withChange(instance.change().withPlatformPin())))); // New version is released Version version1 = Version.fromString("6.2"); @@ -935,7 +933,7 @@ public class UpgraderTest { // App 1 is unpinned and upgrades to latest 6 tester.controller().applications().lockApplicationIfPresent(app1.application().id(), app -> tester.controller().applications().store(app.with(app1.instance().name(), - instance -> instance.withChange(instance.change().withoutPin())))); + instance -> instance.withChange(instance.change().withoutPlatformPin())))); tester.upgrader().maintain(); assertEquals(version1, app1.instance().change().platform().orElseThrow(), diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/maintenance/VcmrMaintainerTest.java b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/maintenance/VcmrMaintainerTest.java index 52bd8e9c618..39bf61df9ed 100644 --- a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/maintenance/VcmrMaintainerTest.java +++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/maintenance/VcmrMaintainerTest.java @@ -58,7 +58,7 @@ public class VcmrMaintainerTest { @Test void recycle_hosts_after_completion() { var vcmrReport = new VcmrReport(); - vcmrReport.addVcmr("id123", ZonedDateTime.now(), ZonedDateTime.now()); + vcmrReport.addVcmr(new ChangeRequestSource("aws", "id123", "url", ChangeRequestSource.Status.WAITING_FOR_APPROVAL , ZonedDateTime.now(), ZonedDateTime.now())); var parkedNode = createNode(host1, NodeType.host, Node.State.parked, true); var failedNode = createNode(host2, NodeType.host, Node.State.failed, false); var reports = vcmrReport.toNodeReports(); @@ -181,7 +181,7 @@ public class VcmrMaintainerTest { activeNode = nodeRepo.list(zoneId, NodeFilter.all().hostnames(host2)).get(0); var report = VcmrReport.fromReports(activeNode.reports()); var reportAdded = report.getVcmrs().stream() - .filter(vcmr -> vcmr.getId().equals(changeRequestId)) + .filter(vcmr -> vcmr.id().equals(changeRequestId)) .count() == 1; assertTrue(reportAdded); } diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/persistence/ApplicationSerializerTest.java b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/persistence/ApplicationSerializerTest.java index 589fc25700f..b71d3cf838b 100644 --- a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/persistence/ApplicationSerializerTest.java +++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/persistence/ApplicationSerializerTest.java @@ -101,16 +101,17 @@ public class ApplicationSerializerTest { Optional.empty(), Optional.of("best commit"), Optional.of("hash1"), + Optional.of(Instant.ofEpochMilli(777)), true, false, Optional.of("~(˘▾˘)~"), 3); assertEquals("https://github/org/repo/tree/commit1", applicationVersion1.sourceUrl().get()); - ApplicationVersion applicationVersion2 = ApplicationVersion.from(RevisionId.forDevelopment(31, new JobId(id1, DeploymentContext.productionUsEast3)), - new SourceRevision("repo1", "branch1", "commit1"), "a@b", - Version.fromString("6.3.1"), - Instant.ofEpochMilli(496)); + RevisionId id = RevisionId.forDevelopment(31, new JobId(id1, DeploymentContext.productionUsEast3)); + SourceRevision source = new SourceRevision("repo1", "branch1", "commit1"); + Version compileVersion = Version.fromString("6.3.1"); + ApplicationVersion applicationVersion2 = new ApplicationVersion(id, Optional.of(source), Optional.of("a@b"), Optional.of(compileVersion), Optional.empty(), Optional.of(Instant.ofEpochMilli(496)), Optional.empty(), Optional.empty(), Optional.empty(), Optional.empty(), true, false, Optional.empty(), 0); Instant activityAt = Instant.parse("2018-06-01T10:15:30.00Z"); deployments.add(new Deployment(zone1, CloudAccount.empty, applicationVersion1.id(), Version.fromString("1.2.3"), Instant.ofEpochMilli(3), DeploymentMetrics.none, DeploymentActivity.none, QuotaUsage.none, OptionalDouble.empty())); @@ -143,7 +144,7 @@ public class ApplicationSerializerTest { Map.of(), List.of(), RotationStatus.EMPTY, - Change.of(Version.fromString("6.7")).withPin())); + Change.of(Version.fromString("6.7")).withPlatformPin().withRevisionPin())); Application original = new Application(TenantAndApplicationId.from(id1), Instant.now().truncatedTo(ChronoUnit.MILLIS), @@ -174,6 +175,7 @@ public class ApplicationSerializerTest { assertEquals(original.revisions().last().get().sourceUrl(), serialized.revisions().last().get().sourceUrl()); assertEquals(original.revisions().last().get().commit(), serialized.revisions().last().get().commit()); assertEquals(original.revisions().last().get().bundleHash(), serialized.revisions().last().get().bundleHash()); + assertEquals(original.revisions().last().get().obsoleteAt(), serialized.revisions().last().get().obsoleteAt()); assertEquals(original.revisions().last().get().hasPackage(), serialized.revisions().last().get().hasPackage()); assertEquals(original.revisions().last().get().shouldSkip(), serialized.revisions().last().get().shouldSkip()); assertEquals(original.revisions().last().get().description(), serialized.revisions().last().get().description()); diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/application/ApplicationApiTest.java b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/application/ApplicationApiTest.java index 9a34989aeff..76bcbe078ff 100644 --- a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/application/ApplicationApiTest.java +++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/application/ApplicationApiTest.java @@ -541,24 +541,22 @@ public class ApplicationApiTest extends ControllerContainerTest { "{\"message\":\"No deployment in progress for tenant1.application1.instance1 at this time\"}"); // POST pinning to a given version to an application - tester.assertResponse(request("/application/v4/tenant/tenant1/application/application1/instance/instance1/deploying/pin", POST) + tester.assertResponse(request("/application/v4/tenant/tenant1/application/application1/instance/instance1/deploying/platform-pin", POST) .userIdentity(USER_ID) .data("6.1.0"), "{\"message\":\"Triggered pin to 6.1 for tenant1.application1.instance1\"}"); assertTrue(tester.controller().auditLogger().readLog().entries().stream() - .anyMatch(entry -> entry.resource().equals("/application/v4/tenant/tenant1/application/application1/instance/instance1/deploying/pin?")), + .anyMatch(entry -> entry.resource().equals("/application/v4/tenant/tenant1/application/application1/instance/instance1/deploying/platform-pin?")), "Action is logged to audit log"); tester.assertResponse(request("/application/v4/tenant/tenant1/application/application1/instance/instance1/deploying", GET) - .userIdentity(USER_ID), "{\"platform\":\"6.1\",\"pinned\":true}"); - tester.assertResponse(request("/application/v4/tenant/tenant1/application/application1/instance/instance1/deploying/pin", GET) - .userIdentity(USER_ID), "{\"platform\":\"6.1\",\"pinned\":true}"); + .userIdentity(USER_ID), "{\"platform\":\"6.1\",\"pinned\":true,\"platform-pinned\":true,\"application-pinned\":false}"); // DELETE only the pin to a given version - tester.assertResponse(request("/application/v4/tenant/tenant1/application/application1/instance/instance1/deploying/pin", DELETE) + tester.assertResponse(request("/application/v4/tenant/tenant1/application/application1/instance/instance1/deploying/platform-pin", DELETE) .userIdentity(USER_ID), "{\"message\":\"Changed deployment from 'pin to 6.1' to 'upgrade to 6.1' for tenant1.application1.instance1\"}"); tester.assertResponse(request("/application/v4/tenant/tenant1/application/application1/instance/instance1/deploying", GET) - .userIdentity(USER_ID), "{\"platform\":\"6.1\",\"pinned\":false}"); + .userIdentity(USER_ID), "{\"platform\":\"6.1\",\"pinned\":false,\"platform-pinned\":false,\"application-pinned\":false}"); // POST pinning again tester.assertResponse(request("/application/v4/tenant/tenant1/application/application1/instance/instance1/deploying/pin", POST) @@ -566,14 +564,14 @@ public class ApplicationApiTest extends ControllerContainerTest { .data("6.1"), "{\"message\":\"Triggered pin to 6.1 for tenant1.application1.instance1\"}"); tester.assertResponse(request("/application/v4/tenant/tenant1/application/application1/instance/instance1/deploying", GET) - .userIdentity(USER_ID), "{\"platform\":\"6.1\",\"pinned\":true}"); + .userIdentity(USER_ID), "{\"platform\":\"6.1\",\"pinned\":true,\"platform-pinned\":true,\"application-pinned\":false}"); // DELETE only the version, but leave the pin tester.assertResponse(request("/application/v4/tenant/tenant1/application/application1/instance/instance1/deploying/platform", DELETE) .userIdentity(USER_ID), "{\"message\":\"Changed deployment from 'pin to 6.1' to 'pin to current platform' for tenant1.application1.instance1\"}"); tester.assertResponse(request("/application/v4/tenant/tenant1/application/application1/instance/instance1/deploying", GET) - .userIdentity(USER_ID), "{\"pinned\":true}"); + .userIdentity(USER_ID), "{\"pinned\":true,\"platform-pinned\":true,\"application-pinned\":false}"); // DELETE also the pin to a given version tester.assertResponse(request("/application/v4/tenant/tenant1/application/application1/instance/instance1/deploying/pin", DELETE) @@ -582,6 +580,32 @@ public class ApplicationApiTest extends ControllerContainerTest { tester.assertResponse(request("/application/v4/tenant/tenant1/application/application1/instance/instance1/deploying", GET) .userIdentity(USER_ID), "{}"); + // POST pinning to a given revision to an application + tester.assertResponse(request("/application/v4/tenant/tenant1/application/application1/instance/instance1/deploying/application-pin", POST) + .userIdentity(USER_ID) + .data(""), + "{\"message\":\"Triggered pin to build 1 for tenant1.application1.instance1\"}"); + assertTrue(tester.controller().auditLogger().readLog().entries().stream() + .anyMatch(entry -> entry.resource().equals("/application/v4/tenant/tenant1/application/application1/instance/instance1/deploying/application-pin?")), + "Action is logged to audit log"); + tester.assertResponse(request("/application/v4/tenant/tenant1/application/application1/instance/instance1/deploying", GET) + .userIdentity(USER_ID), "{\"application\":\"build 1\",\"pinned\":false,\"platform-pinned\":false,\"application-pinned\":true}"); + + // DELETE only the pin to a given revision + tester.assertResponse(request("/application/v4/tenant/tenant1/application/application1/instance/instance1/deploying/application-pin", DELETE) + .userIdentity(USER_ID), + "{\"message\":\"Changed deployment from 'pin to build 1' to 'revision change to build 1' for tenant1.application1.instance1\"}"); + tester.assertResponse(request("/application/v4/tenant/tenant1/application/application1/instance/instance1/deploying", GET) + .userIdentity(USER_ID), "{\"application\":\"build 1\",\"pinned\":false,\"platform-pinned\":false,\"application-pinned\":false}"); + + // DELETE deploying to a given revision + tester.assertResponse(request("/application/v4/tenant/tenant1/application/application1/instance/instance1/deploying/application", DELETE) + .userIdentity(USER_ID), + "{\"message\":\"Changed deployment from 'revision change to build 1' to 'no change' for tenant1.application1.instance1\"}"); + tester.assertResponse(request("/application/v4/tenant/tenant1/application/application1/instance/instance1/deploying", GET) + .userIdentity(USER_ID), "{}"); + + // POST a pause to a production job tester.assertResponse(request("/application/v4/tenant/tenant1/application/application1/instance/instance1/job/production-us-west-1/pause", POST) .userIdentity(USER_ID), diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/application/responses/deployment-overview.json b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/application/responses/deployment-overview.json index ec6ccf3ecf2..0b7c64c72a5 100644 --- a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/application/responses/deployment-overview.json +++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/application/responses/deployment-overview.json @@ -48,6 +48,14 @@ "sourceUrl": "repository1/tree/commit1", "commit": "commit1" } + }, + { + "application": { + "build": 1, + "compileVersion": "6.1.0", + "sourceUrl": "repository1/tree/commit1", + "commit": "commit1" + } } ], "blockers": [ ] @@ -594,6 +602,14 @@ "sourceUrl": "repository1/tree/commit1", "commit": "commit1" } + }, + { + "application": { + "build": 1, + "compileVersion": "6.1.0", + "sourceUrl": "repository1/tree/commit1", + "commit": "commit1" + } } ], "blockers": [ ] @@ -709,6 +725,15 @@ "description": "my best commit yet", "risk": 9001, "deployable": false + }, + { + "build": 1, + "compileVersion": "6.1.0", + "sourceUrl": "repository1/tree/commit1", + "commit": "commit1", + "description": "my best commit yet", + "risk": 9001, + "deployable": true } ] } diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/deployment/responses/root.json b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/deployment/responses/root.json index a1f386d51a7..ac43fbf2a80 100644 --- a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/deployment/responses/root.json +++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/deployment/responses/root.json @@ -37,6 +37,8 @@ "instance": "default", "upgrading": false, "pinned": false, + "platformPinned": false, + "revisionPinned": false, "upgradePolicy": "default", "compileVersion": "6.1.0", "jobs": [ @@ -78,6 +80,8 @@ "instance": "i2", "upgrading": false, "pinned": false, + "platformPinned": false, + "revisionPinned": false, "upgradePolicy": "default", "compileVersion": "6.1.0", "jobs": [ @@ -179,6 +183,8 @@ "instance": "default", "upgrading": true, "pinned": false, + "platformPinned": false, + "revisionPinned": false, "upgradePolicy": "default", "compileVersion": "6.1.0", "jobs": [ @@ -249,6 +255,8 @@ "instance": "i1", "upgrading": false, "pinned": false, + "platformPinned": false, + "revisionPinned": false, "upgradePolicy": "default", "compileVersion": "6.1.0", "jobs": [ @@ -309,6 +317,8 @@ "instance": "i2", "upgrading": true, "pinned": false, + "platformPinned": false, + "revisionPinned": false, "upgradePolicy": "default", "compileVersion": "6.1.0", "jobs": [ diff --git a/docprocs/src/test/cfg/ilscripts.cfg b/docprocs/src/test/cfg/ilscripts.cfg index c58028f1056..6e4c75f46a7 100644 --- a/docprocs/src/test/cfg/ilscripts.cfg +++ b/docprocs/src/test/cfg/ilscripts.cfg @@ -11,3 +11,4 @@ ilscript[0].content[2] "input song | attribute song" ilscript[0].content[4] "input artist . " ". input title | index combined" ilscript[0].content[5] "(input artist || "") . " " . (input title || "") | index combinedWithFallback" + 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 d55b58a1728..ea17d9967ae 100644 --- a/flags/src/main/java/com/yahoo/vespa/flags/Flags.java +++ b/flags/src/main/java/com/yahoo/vespa/flags/Flags.java @@ -398,6 +398,12 @@ public class Flags { "Whether to enable CrowdStrike.", "Takes effect on next host admin tick", HOSTNAME); + public static final UnboundBooleanFlag ALLOW_MORE_THAN_ONE_CONTENT_GROUP_DOWN = defineFeatureFlag( + "allow-more-than-one-content-group-down", false, List.of("hmusum"), "2023-04-14", "2023-06-14", + "Whether to enable possible configuration of letting more than one content group down", + "Takes effect at redeployment", + HOSTNAME); + /** WARNING: public for testing: All flags should be defined in {@link Flags}. */ public static UnboundBooleanFlag defineFeatureFlag(String flagId, boolean defaultValue, List<String> owners, String createdAt, String expiresAt, String description, diff --git a/indexinglanguage/src/main/java/com/yahoo/vespa/indexinglanguage/expressions/ChoiceExpression.java b/indexinglanguage/src/main/java/com/yahoo/vespa/indexinglanguage/expressions/ChoiceExpression.java index 86826770828..5dbb9292a9d 100644 --- a/indexinglanguage/src/main/java/com/yahoo/vespa/indexinglanguage/expressions/ChoiceExpression.java +++ b/indexinglanguage/src/main/java/com/yahoo/vespa/indexinglanguage/expressions/ChoiceExpression.java @@ -50,9 +50,9 @@ public class ChoiceExpression extends ExpressionList<Expression> { @Override protected void doVerify(VerificationContext context) { DataType input = context.getValueType(); + context.setValueType(input); for (Expression exp : this) context.setValueType(input).execute(exp); - context.setValueType(input); } private static DataType resolveInputType(Collection<? extends Expression> list) { diff --git a/indexinglanguage/src/main/java/com/yahoo/vespa/indexinglanguage/expressions/VerificationContext.java b/indexinglanguage/src/main/java/com/yahoo/vespa/indexinglanguage/expressions/VerificationContext.java index c667a0019c2..fb1338b8b65 100644 --- a/indexinglanguage/src/main/java/com/yahoo/vespa/indexinglanguage/expressions/VerificationContext.java +++ b/indexinglanguage/src/main/java/com/yahoo/vespa/indexinglanguage/expressions/VerificationContext.java @@ -11,7 +11,7 @@ import java.util.Map; */ public class VerificationContext implements FieldTypeAdapter, Cloneable { - private final Map<String, DataType> variables = new HashMap<String, DataType>(); + private final Map<String, DataType> variables = new HashMap<>(); private final FieldTypeAdapter adapter; private DataType value; private String outputField; diff --git a/indexinglanguage/src/test/java/com/yahoo/vespa/indexinglanguage/expressions/ChoiceTestCase.java b/indexinglanguage/src/test/java/com/yahoo/vespa/indexinglanguage/expressions/ChoiceTestCase.java index 7ece841e9b7..e6d5c550e93 100644 --- a/indexinglanguage/src/test/java/com/yahoo/vespa/indexinglanguage/expressions/ChoiceTestCase.java +++ b/indexinglanguage/src/test/java/com/yahoo/vespa/indexinglanguage/expressions/ChoiceTestCase.java @@ -2,7 +2,9 @@ package com.yahoo.vespa.indexinglanguage.expressions; import com.yahoo.document.DataType; +import com.yahoo.document.Document; import com.yahoo.document.Field; +import com.yahoo.document.datatypes.LongFieldValue; import com.yahoo.document.datatypes.StringFieldValue; import com.yahoo.language.Linguistics; import com.yahoo.language.process.Embedder; @@ -44,6 +46,7 @@ public class ChoiceTestCase { var adapter = new SimpleTestAdapter(new Field("foo", DataType.STRING), new Field("bar", DataType.STRING)); adapter.setValue("foo", new StringFieldValue("foo1")); adapter.setValue("bar", new StringFieldValue("bar1")); + choice.verify(adapter); ExecutionContext context = new ExecutionContext(adapter); choice.execute(context); assertEquals("foo1", context.getValue().getWrappedValue()); @@ -51,6 +54,28 @@ public class ChoiceTestCase { } @Test + public void testChoiceWithConstant() throws ParseException { + var choice = parse("input timestamp || 99999999L | attribute timestamp"); + + { // value is set + var adapter = new SimpleTestAdapter(new Field("timestamp", DataType.LONG)); + choice.verify(adapter); + adapter.setValue("timestamp", new LongFieldValue(34)); + ExecutionContext context = new ExecutionContext(adapter); + choice.execute(context); + assertEquals(34L, context.getValue().getWrappedValue()); + } + + { // fallback to default + var adapter = new SimpleTestAdapter(new Field("timestamp", DataType.LONG)); + choice.verify(adapter); + ExecutionContext context = new ExecutionContext(adapter); + choice.execute(context); + assertEquals(99999999L, context.getValue().getWrappedValue()); + } + } + + @Test public void testIllegalChoiceExpression() throws ParseException { try { parse("input (foo || 99999999) | attribute"); diff --git a/model-integration/src/main/java/ai/vespa/embedding/BertBaseEmbedder.java b/model-integration/src/main/java/ai/vespa/embedding/BertBaseEmbedder.java index 19536f3cb32..bf56d233f89 100644 --- a/model-integration/src/main/java/ai/vespa/embedding/BertBaseEmbedder.java +++ b/model-integration/src/main/java/ai/vespa/embedding/BertBaseEmbedder.java @@ -48,7 +48,7 @@ public class BertBaseEmbedder extends AbstractComponent implements Embedder { public BertBaseEmbedder(OnnxRuntime onnx, BertBaseEmbedderConfig config) { maxTokens = config.transformerMaxTokens(); startSequenceToken = config.transformerStartSequenceToken(); - endSequenceToken = config.transformerStartSequenceToken(); + endSequenceToken = config.transformerEndSequenceToken(); inputIdsName = config.transformerInputIds(); attentionMaskName = config.transformerAttentionMask(); tokenTypeIdsName = config.transformerTokenTypeIds(); diff --git a/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/task/util/network/IPAddressesImpl.java b/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/task/util/network/IPAddressesImpl.java index 1b5e6c57f5c..b7a1cb8a16e 100644 --- a/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/task/util/network/IPAddressesImpl.java +++ b/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/task/util/network/IPAddressesImpl.java @@ -1,6 +1,7 @@ // Copyright Yahoo. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. package com.yahoo.vespa.hosted.node.admin.task.util.network; +import java.io.UncheckedIOException; import java.net.InetAddress; import java.net.UnknownHostException; @@ -14,7 +15,7 @@ public class IPAddressesImpl implements IPAddresses { try { return InetAddress.getAllByName(hostname); } catch (UnknownHostException e) { - throw new RuntimeException(e); + throw new UncheckedIOException(e); } } } diff --git a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/maintenance/NodeFailer.java b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/maintenance/NodeFailer.java index f9ff2f08375..766bc688c62 100644 --- a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/maintenance/NodeFailer.java +++ b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/maintenance/NodeFailer.java @@ -7,6 +7,7 @@ import com.yahoo.config.provision.Deployment; import com.yahoo.config.provision.NodeType; import com.yahoo.config.provision.TransientException; import com.yahoo.jdisc.Metric; +import com.yahoo.slime.SlimeUtils; import com.yahoo.transaction.Mutex; import com.yahoo.vespa.hosted.provision.Node; import com.yahoo.vespa.hosted.provision.NodeList; @@ -109,7 +110,7 @@ public class NodeFailer extends NodeRepositoryMaintainer { for (Node node : activeNodes) { Instant graceTimeStart = clock().instant().minus(nodeRepository().nodes().suspended(node) ? suspendedDownTimeLimit : downTimeLimit); - if (node.isDown() && node.history().hasEventBefore(History.Event.Type.down, graceTimeStart) && !applicationSuspended(node)) { + if (node.isDown() && node.history().hasEventBefore(History.Event.Type.down, graceTimeStart) && !applicationSuspended(node) && !undergoingCmr(node)) { // Allow a grace period after node re-activation if (!node.history().hasEventAfter(History.Event.Type.activated, graceTimeStart)) failingNodes.add(new FailingNode(node, "Node has been down longer than " + downTimeLimit)); @@ -157,6 +158,19 @@ public class NodeFailer extends NodeRepositoryMaintainer { } } + private boolean undergoingCmr(Node node) { + return node.reports().getReport("vcmr") + .map(report -> + SlimeUtils.entriesStream(report.getInspector().field("upcoming")) + .anyMatch(cmr -> { + var startTime = cmr.field("plannedStartTime").asLong(); + var endTime = cmr.field("plannedEndTime").asLong(); + var now = clock().instant().getEpochSecond(); + return now > startTime && now < endTime; + }) + ).orElse(false); + } + /** Is the node and all active children suspended? */ private boolean allSuspended(Node node, NodeList activeNodes) { if (!nodeRepository().nodes().suspended(node)) return false; diff --git a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/provisioning/GroupPreparer.java b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/provisioning/GroupPreparer.java index eff39dddf42..06c1916dd4f 100644 --- a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/provisioning/GroupPreparer.java +++ b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/provisioning/GroupPreparer.java @@ -21,7 +21,6 @@ import java.util.function.Consumer; import java.util.function.Supplier; import java.util.logging.Level; import java.util.logging.Logger; -import java.util.stream.Collectors; /** * Performs preparation of node activation changes for a single host group in an application. @@ -124,6 +123,14 @@ public class GroupPreparer { hosts.forEach(host -> nodeRepository.nodes().deprovision(host.hostname(), Agent.system, nodeRepository.clock().instant())); throw e; } + } else if (allocation.hostDeficit().isPresent() && requestedNodes.canFail() && + allocation.hasRetiredJustNow() && requestedNodes instanceof NodeSpec.CountNodeSpec cns) { + // Non-dynamically provisioned zone with a deficit because we just now retired some nodes. + // Try again, but without retiring + indices.resetProbe(); + List<Node> accepted = prepareWithLocks(application, cluster, cns.withoutRetiring(), surplusActiveNodes, indices, wantedGroups); + log.warning("Prepared " + application + " " + cluster.id() + " without retirement due to lack of capacity"); + return accepted; } if (! allocation.fulfilled() && requestedNodes.canFail()) diff --git a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/provisioning/LoadBalancerProvisioner.java b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/provisioning/LoadBalancerProvisioner.java index 890d190c24e..b3198a72d1b 100644 --- a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/provisioning/LoadBalancerProvisioner.java +++ b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/provisioning/LoadBalancerProvisioner.java @@ -193,8 +193,8 @@ public class LoadBalancerProvisioner { Optional<LoadBalancer> loadBalancer = db.readLoadBalancer(id); LoadBalancer newLoadBalancer; LoadBalancer.State fromState = loadBalancer.map(LoadBalancer::state).orElse(null); - boolean recreateLoadBalancer = loadBalancer.isPresent() && (!inAccount(cloudAccount, loadBalancer.get()) - || !hasCorrectVisibility(loadBalancer.get(), zoneEndpoint)); + boolean recreateLoadBalancer = loadBalancer.isPresent() && ( ! inAccount(cloudAccount, loadBalancer.get()) + || ! hasCorrectVisibility(loadBalancer.get(), zoneEndpoint)); if (recreateLoadBalancer) { // We have a load balancer, but with the wrong account or visibility. // Load balancer must be removed before we can provision a new one with the wanted visibility diff --git a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/provisioning/NodeAllocation.java b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/provisioning/NodeAllocation.java index c6971f0fe02..3af63125474 100644 --- a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/provisioning/NodeAllocation.java +++ b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/provisioning/NodeAllocation.java @@ -307,10 +307,15 @@ class NodeAllocation { } /** Returns true if this allocation was already fulfilled and resulted in no new changes */ - public boolean fulfilledAndNoChanges() { + boolean fulfilledAndNoChanges() { return fulfilled() && reservableNodes().isEmpty() && newNodes().isEmpty(); } + /** Returns true if this allocation has retired nodes */ + boolean hasRetiredJustNow() { + return wasRetiredJustNow > 0; + } + /** * Returns {@link HostDeficit} describing the host deficit for the given {@link NodeSpec}. * @@ -451,7 +456,7 @@ class NodeAllocation { .toList(); } - public String allocationFailureDetails() { + String allocationFailureDetails() { List<String> reasons = new ArrayList<>(); if (rejectedDueToExclusivity > 0) reasons.add("host exclusivity constraints"); diff --git a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/provisioning/NodeSpec.java b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/provisioning/NodeSpec.java index 9edfa221abf..28d1e7c1c68 100644 --- a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/provisioning/NodeSpec.java +++ b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/provisioning/NodeSpec.java @@ -3,8 +3,6 @@ package com.yahoo.vespa.hosted.provision.provisioning; import com.yahoo.config.provision.CloudAccount; import com.yahoo.config.provision.ClusterSpec; -import com.yahoo.config.provision.Flavor; -import com.yahoo.config.provision.NodeFlavors; import com.yahoo.config.provision.NodeResources; import com.yahoo.config.provision.NodeType; import com.yahoo.vespa.hosted.provision.Node; @@ -79,7 +77,7 @@ public interface NodeSpec { } static NodeSpec from(int nodeCount, NodeResources resources, boolean exclusive, boolean canFail, CloudAccount cloudAccount) { - return new CountNodeSpec(nodeCount, resources, exclusive, canFail, cloudAccount); + return new CountNodeSpec(nodeCount, resources, exclusive, canFail, canFail, cloudAccount); } static NodeSpec from(NodeType type, CloudAccount cloudAccount) { @@ -93,14 +91,19 @@ public interface NodeSpec { private final NodeResources requestedNodeResources; private final boolean exclusive; private final boolean canFail; + private final boolean considerRetiring; private final CloudAccount cloudAccount; - private CountNodeSpec(int count, NodeResources resources, boolean exclusive, boolean canFail, CloudAccount cloudAccount) { + private CountNodeSpec(int count, NodeResources resources, boolean exclusive, boolean canFail, boolean considerRetiring, CloudAccount cloudAccount) { this.count = count; this.requestedNodeResources = Objects.requireNonNull(resources, "Resources must be specified"); this.exclusive = exclusive; this.canFail = canFail; + this.considerRetiring = considerRetiring; this.cloudAccount = Objects.requireNonNull(cloudAccount); + + if (!canFail && considerRetiring) + throw new IllegalArgumentException("Cannot consider retiring nodes if we cannot fail"); } @Override @@ -127,8 +130,7 @@ public interface NodeSpec { @Override public boolean considerRetiring() { - // If we cannot fail we cannot retire as we may end up without sufficient replacement capacity - return canFail(); + return considerRetiring; } @Override @@ -143,7 +145,11 @@ public interface NodeSpec { @Override public NodeSpec fraction(int divisor) { - return new CountNodeSpec(count/divisor, requestedNodeResources, exclusive, canFail, cloudAccount); + return new CountNodeSpec(count/divisor, requestedNodeResources, exclusive, canFail, considerRetiring, cloudAccount); + } + + public NodeSpec withoutRetiring() { + return new CountNodeSpec(count, requestedNodeResources, exclusive, canFail, false, cloudAccount); } @Override diff --git a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/maintenance/NodeFailerTest.java b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/maintenance/NodeFailerTest.java index 491485b78fc..c63be6d5dc5 100644 --- a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/maintenance/NodeFailerTest.java +++ b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/maintenance/NodeFailerTest.java @@ -6,6 +6,7 @@ import com.yahoo.config.provision.ClusterResources; import com.yahoo.config.provision.ClusterSpec; import com.yahoo.config.provision.NodeResources; import com.yahoo.config.provision.NodeType; +import com.yahoo.slime.SlimeUtils; import com.yahoo.vespa.applicationmodel.ServiceInstance; import com.yahoo.vespa.applicationmodel.ServiceStatus; import com.yahoo.vespa.hosted.provision.Node; @@ -627,6 +628,49 @@ public class NodeFailerTest { assertFalse(badNode(1, 3, 1, 2)); } + @Test + public void nodes_undergoing_cmr_are_not_failed() { + var tester = NodeFailTester.withTwoApplications(6); + var clock = tester.clock; + var slime = SlimeUtils.jsonToSlime( + String.format(""" + { + "upcoming":[{ + "id": "id-42", + "status": "some-status", + "plannedStartTime": %d, + "plannedEndTime": %d + }] + } + """, clock.instant().getEpochSecond(), clock.instant().plus(Duration.ofMinutes(90)).getEpochSecond()) + ); + var cmrReport = Report.fromSlime("vcmr", slime.get()); + var downHost = tester.nodeRepository.nodes().list(Node.State.active).owner(NodeFailTester.app1).asList().get(1).hostname(); + + var node = tester.nodeRepository.nodes().node(downHost).get(); + tester.nodeRepository.nodes().write(node.with(node.reports().withReport(cmrReport)), () -> {}); + + tester.serviceMonitor.setHostDown(downHost); + tester.runMaintainers(); + node = tester.nodeRepository.nodes().node(downHost).get(); + assertTrue(node.isDown()); + assertEquals(Node.State.active, node.state()); + + // CMR still ongoing, don't fail yet + clock.advance(Duration.ofHours(1)); + tester.runMaintainers(); + node = tester.nodeRepository.nodes().node(downHost).get(); + assertTrue(node.isDown()); + assertEquals(Node.State.active, node.state()); + + // No ongoing CMR anymore, host should be failed + clock.advance(Duration.ofHours(1)); + tester.runMaintainers(); + node = tester.nodeRepository.nodes().node(downHost).get(); + assertTrue(node.isDown()); + assertEquals(Node.State.failed, node.state()); + } + private void addServiceInstances(List<ServiceInstance> list, ServiceStatus status, int num) { for (int i = 0; i < num; ++i) { ServiceInstance service = mock(ServiceInstance.class); diff --git a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/provisioning/ProvisioningTest.java b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/provisioning/ProvisioningTest.java index 978edf3f7e4..2cd0e84c356 100644 --- a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/provisioning/ProvisioningTest.java +++ b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/provisioning/ProvisioningTest.java @@ -757,6 +757,23 @@ public class ProvisioningTest { } @Test + public void ignore_retirement_if_no_capacity() { + ProvisioningTester tester = new ProvisioningTester.Builder().zone(new Zone(Environment.prod, RegionName.from("us-east"))).build(); + tester.makeReadyHosts(3, defaultResources).activateTenantHosts(); + + ApplicationId application = ProvisioningTester.applicationId(); + ClusterSpec cluster = ClusterSpec.request(ClusterSpec.Type.content, ClusterSpec.Id.from("music")).vespaVersion("4.5.6").build(); + tester.activate(application, tester.prepare(application, cluster, 3, 1, defaultResources)); + + // Mark the nodes as want to retire + NodeList nodes = tester.getNodes(application); + tester.patchNodes(nodes.asList(), node -> node.withWantToRetire(true, Agent.system, tester.clock().instant())); + + tester.activate(application, tester.prepare(application, cluster, 3, 1, defaultResources)); + assertEquals(3, tester.getNodes(application).state(Node.State.active).not().retired().size()); + } + + @Test public void highest_node_indexes_are_retired_first() { ProvisioningTester tester = new ProvisioningTester.Builder().zone(new Zone(Environment.prod, RegionName.from("us-east"))).build(); diff --git a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/provisioning/VirtualNodeProvisioningTest.java b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/provisioning/VirtualNodeProvisioningTest.java index fb773f19b8a..0744d82c85b 100644 --- a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/provisioning/VirtualNodeProvisioningTest.java +++ b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/provisioning/VirtualNodeProvisioningTest.java @@ -34,7 +34,6 @@ import java.util.stream.Stream; import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertFalse; -import static org.junit.Assert.assertNotNull; import static org.junit.Assert.assertTrue; import static org.junit.Assert.fail; @@ -171,7 +170,7 @@ public class VirtualNodeProvisioningTest { @Test public void indistinct_distribution_with_known_ready_nodes() { ProvisioningTester tester = new ProvisioningTester.Builder().build(); - tester.makeReadyChildren(3, resources1); + tester.makeReadyChildren(4, resources1); int contentNodeCount = 3; int groups = 1; @@ -191,13 +190,10 @@ public class VirtualNodeProvisioningTest { tester.makeReadyChildren(1, resources1, "parentHost1"); tester.makeReadyChildren(2, resources1, "parentHost2"); - NodeAllocationException expectedException = null; - try { - tester.prepare(applicationId, contentClusterSpec, contentNodeCount, groups, resources1); - } catch (NodeAllocationException e) { - expectedException = e; - } - assertNotNull(expectedException); + tester.activate(applicationId, tester.prepare(applicationId, contentClusterSpec, contentNodeCount, groups, resources1)); + nodes = tester.getNodes(applicationId, Node.State.active); + assertEquals(4, nodes.size()); + assertEquals(1, nodes.retired().size()); } @Test diff --git a/parent/pom.xml b/parent/pom.xml index ace6a47d6bc..8d2f802e34b 100644 --- a/parent/pom.xml +++ b/parent/pom.xml @@ -1166,7 +1166,7 @@ <netty.version>4.1.86.Final</netty.version> <netty-tcnative.version>2.0.54.Final</netty-tcnative.version> <onnxruntime.version>1.13.1</onnxruntime.version> <!-- WARNING: sync cloud-tenant-base-dependencies-enforcer/pom.xml --> - <org.json.version>20220320</org.json.version> + <org.json.version>20230227</org.json.version> <org.lz4.version>1.8.0</org.lz4.version> <prometheus.client.version>0.6.0</prometheus.client.version> <protobuf.version>3.21.7</protobuf.version> diff --git a/searchcore/src/tests/proton/attribute/attribute_initializer/attribute_initializer_test.cpp b/searchcore/src/tests/proton/attribute/attribute_initializer/attribute_initializer_test.cpp index 52b7a7ce60a..4af23a1d7fb 100644 --- a/searchcore/src/tests/proton/attribute/attribute_initializer/attribute_initializer_test.cpp +++ b/searchcore/src/tests/proton/attribute/attribute_initializer/attribute_initializer_test.cpp @@ -7,8 +7,11 @@ #include <vespa/searchcore/proton/test/attribute_utils.h> #include <vespa/searchlib/attribute/attributefactory.h> #include <vespa/searchlib/attribute/integerbase.h> +#include <vespa/searchlib/attribute/stringbase.h> #include <vespa/searchlib/test/directory_handler.h> #include <vespa/searchcommon/attribute/config.h> +#include <vespa/searchcommon/attribute/i_multi_value_attribute.h> +#include <vespa/vespalib/util/stash.h> #include <vespa/vespalib/util/threadstackexecutor.h> #include <vespa/vespalib/testkit/testapp.h> @@ -18,8 +21,10 @@ LOG_SETUP("attribute_initializer_test"); using search::attribute::Config; using search::attribute::BasicType; using search::attribute::CollectionType; +using search::attribute::IMultiValueAttribute; using search::SerialNum; using search::test::DirectoryHandler; +using vespalib::Stash; const vespalib::string test_dir = "test_output"; @@ -60,7 +65,7 @@ Config get_int32_wset_fs() } void -saveAttr(const vespalib::string &name, const Config &cfg, SerialNum serialNum, SerialNum createSerialNum) +saveAttr(const vespalib::string &name, const Config &cfg, SerialNum serialNum, SerialNum createSerialNum, bool mutate_reserved_doc = false) { auto diskLayout = AttributeDiskLayout::create(test_dir); auto dir = diskLayout->createAttributeDir(name); @@ -81,6 +86,15 @@ saveAttr(const vespalib::string &name, const Config &cfg, SerialNum serialNum, S iav.append(docId, 10, 1); iav.append(docId, 11, 1); } + if (mutate_reserved_doc) { + av->clearDoc(0u); + if (cfg.basicType().type() == BasicType::Type::STRING && + cfg.collectionType().type() == CollectionType::Type::WSET) { + auto &sav = dynamic_cast<search::StringAttribute &>(*av); + sav.append(0u, "badly", 15); + sav.append(0u, "broken", 20); + } + } av->save(); writer->markValidSnapshot(serialNum); } @@ -250,6 +264,26 @@ TEST("require that saved attribute is ignored when serial num is not set") EXPECT_EQUAL(1u, av->getNumDocs()); } +TEST("require that reserved document is reinitialized during load") +{ + saveAttr("a", string_wset, 10, 2, true); + Fixture f; + auto av = f.createInitializer({"a", string_wset}, 5)->init().getAttribute(); + EXPECT_EQUAL(2u, av->getCreateSerialNum()); + EXPECT_EQUAL(2u, av->getNumDocs()); + auto mvav = av->as_multi_value_attribute(); + ASSERT_TRUE(mvav != nullptr); + Stash stash; + auto read_view = mvav->make_read_view(IMultiValueAttribute::WeightedSetTag<const char*>(), stash); + ASSERT_TRUE(read_view != nullptr); + auto reserved_values = read_view->get_values(0u); + EXPECT_EQUAL(1u, reserved_values.size()); + if (reserved_values.size() >= 1) { + EXPECT_EQUAL(1, reserved_values[0].weight()); + EXPECT_EQUAL(vespalib::string(""), vespalib::string(reserved_values[0].value())); + } +} + } TEST_MAIN() diff --git a/searchcore/src/vespa/searchcore/proton/attribute/attribute_initializer.cpp b/searchcore/src/vespa/searchcore/proton/attribute/attribute_initializer.cpp index d39d2873edb..f6200bb14ee 100644 --- a/searchcore/src/vespa/searchcore/proton/attribute/attribute_initializer.cpp +++ b/searchcore/src/vespa/searchcore/proton/attribute/attribute_initializer.cpp @@ -200,6 +200,7 @@ AttributeInitializer::loadAttribute(const AttributeVectorSP &attr, attr->getBaseFileName().c_str()); return false; } else { + attr->set_reserved_doc_values(); attr->commit(CommitParam(serialNum)); EventLogger::loadAttributeComplete(_documentSubDbName, attr->getName(), timer.elapsed()); } diff --git a/searchlib/src/main/java/com/yahoo/searchlib/expression/RawResultNode.java b/searchlib/src/main/java/com/yahoo/searchlib/expression/RawResultNode.java index 5a0e056f254..d1dc46fc4d0 100644 --- a/searchlib/src/main/java/com/yahoo/searchlib/expression/RawResultNode.java +++ b/searchlib/src/main/java/com/yahoo/searchlib/expression/RawResultNode.java @@ -18,8 +18,8 @@ public class RawResultNode extends SingleResultNode { // The global class identifier shared with C++. public static final int classId = registerClass(0x4000 + 54, RawResultNode.class); - private static RawResultNode negativeInfinity = new RawResultNode(); - private static PositiveInfinityResultNode positiveInfinity = new PositiveInfinityResultNode(); + private static final RawResultNode negativeInfinity = new RawResultNode(); + private static final PositiveInfinityResultNode positiveInfinity = new PositiveInfinityResultNode(); // The raw value of this node. private RawData value = null; @@ -147,7 +147,7 @@ public class RawResultNode extends SingleResultNode { @Override public Object getValue() { - return getString(); + return value; } @Override diff --git a/searchlib/src/tests/attribute/attribute_test.cpp b/searchlib/src/tests/attribute/attribute_test.cpp index d2d3ccaad23..3a1a5b457ef 100644 --- a/searchlib/src/tests/attribute/attribute_test.cpp +++ b/searchlib/src/tests/attribute/attribute_test.cpp @@ -913,7 +913,7 @@ AttributeTest::testSingle() AttributePtr ptr = createAttribute("sv-post-int32", cfg); ptr->updateStat(true); EXPECT_EQ(338972u, ptr->getStatus().getAllocated()); - EXPECT_EQ(101492u, ptr->getStatus().getUsed()); + EXPECT_EQ(101632u, ptr->getStatus().getUsed()); addDocs(ptr, numDocs); testSingle<IntegerAttribute, AttributeVector::largeint_t, int32_t>(ptr, values); } @@ -935,7 +935,7 @@ AttributeTest::testSingle() AttributePtr ptr = createAttribute("sv-post-float", cfg); ptr->updateStat(true); EXPECT_EQ(338972u, ptr->getStatus().getAllocated()); - EXPECT_EQ(101492u, ptr->getStatus().getUsed()); + EXPECT_EQ(101632u, ptr->getStatus().getUsed()); addDocs(ptr, numDocs); testSingle<FloatingPointAttribute, double, float>(ptr, values); } @@ -948,7 +948,7 @@ AttributeTest::testSingle() AttributePtr ptr = createAttribute("sv-string", Config(BasicType::STRING, CollectionType::SINGLE)); ptr->updateStat(true); EXPECT_EQ(116528u + sizeof_large_string_entry, ptr->getStatus().getAllocated()); - EXPECT_EQ(52760u + sizeof_large_string_entry, ptr->getStatus().getUsed()); + EXPECT_EQ(52844u + sizeof_large_string_entry, ptr->getStatus().getUsed()); addDocs(ptr, numDocs); testSingle<StringAttribute, string, string>(ptr, values); } @@ -958,7 +958,7 @@ AttributeTest::testSingle() AttributePtr ptr = createAttribute("sv-fs-string", cfg); ptr->updateStat(true); EXPECT_EQ(344848u + sizeof_large_string_entry, ptr->getStatus().getAllocated()); - EXPECT_EQ(104408u + sizeof_large_string_entry, ptr->getStatus().getUsed()); + EXPECT_EQ(104556u + sizeof_large_string_entry, ptr->getStatus().getUsed()); addDocs(ptr, numDocs); testSingle<StringAttribute, string, string>(ptr, values); } @@ -1110,7 +1110,7 @@ AttributeTest::testArray() AttributePtr ptr = createAttribute("a-fs-int32", cfg); ptr->updateStat(true); EXPECT_EQ(844116u, ptr->getStatus().getAllocated()); - EXPECT_EQ(581232u, ptr->getStatus().getUsed()); + EXPECT_EQ(581372u, ptr->getStatus().getUsed()); addDocs(ptr, numDocs); testArray<IntegerAttribute, AttributeVector::largeint_t>(ptr, values); } @@ -1129,7 +1129,7 @@ AttributeTest::testArray() AttributePtr ptr = createAttribute("a-fs-float", cfg); ptr->updateStat(true); EXPECT_EQ(844116u, ptr->getStatus().getAllocated()); - EXPECT_EQ(581232u, ptr->getStatus().getUsed()); + EXPECT_EQ(581372u, ptr->getStatus().getUsed()); addDocs(ptr, numDocs); testArray<FloatingPointAttribute, double>(ptr, values); } @@ -1141,7 +1141,7 @@ AttributeTest::testArray() AttributePtr ptr = createAttribute("a-string", Config(BasicType::STRING, CollectionType::ARRAY)); ptr->updateStat(true); EXPECT_EQ(599784u + sizeof_large_string_entry, ptr->getStatus().getAllocated()); - EXPECT_EQ(532480u + sizeof_large_string_entry, ptr->getStatus().getUsed()); + EXPECT_EQ(532564u + sizeof_large_string_entry, ptr->getStatus().getUsed()); addDocs(ptr, numDocs); testArray<StringAttribute, string>(ptr, values); } @@ -1151,7 +1151,7 @@ AttributeTest::testArray() AttributePtr ptr = createAttribute("afs-string", cfg); ptr->updateStat(true); EXPECT_EQ(849992u + sizeof_large_string_entry, ptr->getStatus().getAllocated()); - EXPECT_EQ(584148u + sizeof_large_string_entry, ptr->getStatus().getUsed()); + EXPECT_EQ(584296u + sizeof_large_string_entry, ptr->getStatus().getUsed()); addDocs(ptr, numDocs); testArray<StringAttribute, string>(ptr, values); } @@ -1718,7 +1718,7 @@ AttributeTest::testStatus() ptr->commit(true); EXPECT_EQ(ptr->getStatus().getNumDocs(), 100u); EXPECT_EQ(ptr->getStatus().getNumValues(), 100u); - EXPECT_EQ(ptr->getStatus().getNumUniqueValues(), 1u); + EXPECT_EQ(ptr->getStatus().getNumUniqueValues(), 2u); size_t expUsed = 0; expUsed += 1 * InternalNodeSize + 1 * LeafNodeSize; // enum store tree expUsed += 1 * 32; // enum store (uniquevalues * bytes per entry) @@ -1741,7 +1741,7 @@ AttributeTest::testStatus() ptr->commit(true); EXPECT_EQ(ptr->getStatus().getNumDocs(), numDocs); EXPECT_EQ(ptr->getStatus().getNumValues(), numDocs*numValuesPerDoc); - EXPECT_EQ(ptr->getStatus().getNumUniqueValues(), numUniq); + EXPECT_EQ(ptr->getStatus().getNumUniqueValues(), numUniq + 1); size_t expUsed = 0; expUsed += 1 * InternalNodeSize + 1 * LeafNodeSize; // Approximate enum store tree expUsed += 272; // TODO Approximate... enum store (16 unique values, 17 bytes per entry) @@ -2145,12 +2145,12 @@ AttributeTest::test_default_value_ref_count_is_updated_after_shrink_lid_space() const auto & iattr = dynamic_cast<const search::IntegerAttributeTemplate<int32_t> &>(*attr); attr->addReservedDoc(); attr->addDocs(10); - EXPECT_EQ(11u, get_default_value_ref_count(*attr, iattr.defaultValue())); + EXPECT_EQ(12u, get_default_value_ref_count(*attr, iattr.defaultValue())); attr->compactLidSpace(6); - EXPECT_EQ(11u, get_default_value_ref_count(*attr, iattr.defaultValue())); + EXPECT_EQ(12u, get_default_value_ref_count(*attr, iattr.defaultValue())); attr->shrinkLidSpace(); EXPECT_EQ(6u, attr->getNumDocs()); - EXPECT_EQ(6u, get_default_value_ref_count(*attr, iattr.defaultValue())); + EXPECT_EQ(7u, get_default_value_ref_count(*attr, iattr.defaultValue())); } template <typename AttributeType> @@ -2170,7 +2170,7 @@ AttributeTest::requireThatAddressSpaceUsageIsReported(const Config &config, bool AddressSpaceUsage after = attrPtr->getAddressSpaceUsage(); if (attrPtr->hasEnum()) { LOG(info, "requireThatAddressSpaceUsageIsReported(%s): Has enum", attrName.c_str()); - EXPECT_EQ(before.enum_store_usage().used(), 1u); + EXPECT_EQ(before.enum_store_usage().used(), 2u); EXPECT_EQ(before.enum_store_usage().dead(), 1u); EXPECT_GT(after.enum_store_usage().used(), before.enum_store_usage().used()); EXPECT_GE(after.enum_store_usage().limit(), before.enum_store_usage().limit()); diff --git a/searchlib/src/tests/attribute/enumeratedsave/enumeratedsave_test.cpp b/searchlib/src/tests/attribute/enumeratedsave/enumeratedsave_test.cpp index 820f39089d1..5501c99652b 100644 --- a/searchlib/src/tests/attribute/enumeratedsave/enumeratedsave_test.cpp +++ b/searchlib/src/tests/attribute/enumeratedsave/enumeratedsave_test.cpp @@ -183,7 +183,7 @@ MemAttr::bufEqual(const Buffer &lhs, const Buffer &rhs) const return false; if (lhs.get() == NULL) return true; - if (!EXPECT_TRUE(lhs->getDataLen() == rhs->getDataLen())) + if (!EXPECT_EQUAL(lhs->getDataLen(), rhs->getDataLen())) return false; if (!EXPECT_TRUE(vespalib::memcmp_safe(lhs->getData(), rhs->getData(), lhs->getDataLen()) == 0)) @@ -243,8 +243,9 @@ EnumeratedSaveTest::populate(IntegerAttribute &v, unsigned seed, int weight = 1; for(size_t i(0), m(v.getNumDocs()); i < m; i++) { v.clearDoc(i); - if (i == 9) + if (i == 9) { continue; + } if (i == 7) { if (v.hasMultiValue()) { v.append(i, -42, 27); @@ -270,7 +271,7 @@ EnumeratedSaveTest::populate(IntegerAttribute &v, unsigned seed, i + 1); } } else { - EXPECT_TRUE( v.update(i, lrand48() & mask) ); + EXPECT_TRUE( v.update(i, rnd.lrand48() & mask) ); } } v.commit(); @@ -288,8 +289,9 @@ EnumeratedSaveTest::populate(FloatingPointAttribute &v, unsigned seed, int weight = 1; for(size_t i(0), m(v.getNumDocs()); i < m; i++) { v.clearDoc(i); - if (i == 9) + if (i == 9) { continue; + } if (i == 7) { if (v.hasMultiValue()) { v.append(i, -42.0, 27); @@ -315,7 +317,7 @@ EnumeratedSaveTest::populate(FloatingPointAttribute &v, unsigned seed, i + 1); } } else { - EXPECT_TRUE( v.update(i, lrand48()) ); + EXPECT_TRUE( v.update(i, rnd.lrand48()) ); } } v.commit(); @@ -332,8 +334,9 @@ EnumeratedSaveTest::populate(StringAttribute &v, unsigned seed, int weight = 1; for(size_t i(0), m(v.getNumDocs()); i < m; i++) { v.clearDoc(i); - if (i == 9) + if (i == 9) { continue; + } if (i == 7) { if (v.hasMultiValue()) { v.append(i, "foo", 27); @@ -712,9 +715,9 @@ EnumeratedSaveTest::test(BasicType bt, CollectionType ct, Config check_cfg(cfg); check_cfg.setFastSearch(true); - checkLoad<VectorType, BufferType>(check_cfg, pref + "0_ee", v0); - checkLoad<VectorType, BufferType>(check_cfg, pref + "1_ee", v1); - checkLoad<VectorType, BufferType>(check_cfg, pref + "2_ee", v2); + TEST_DO((checkLoad<VectorType, BufferType>(check_cfg, pref + "0_ee", v0))); + TEST_DO((checkLoad<VectorType, BufferType>(check_cfg, pref + "1_ee", v1))); + TEST_DO((checkLoad<VectorType, BufferType>(check_cfg, pref + "2_ee", v2))); TEST_DO((testReload<VectorType, BufferType>(v0, v1, v2, mv0, mv1, mv2, diff --git a/searchlib/src/tests/attribute/enumstore/enumstore_test.cpp b/searchlib/src/tests/attribute/enumstore/enumstore_test.cpp index b3c7516777c..2b01c266e80 100644 --- a/searchlib/src/tests/attribute/enumstore/enumstore_test.cpp +++ b/searchlib/src/tests/attribute/enumstore/enumstore_test.cpp @@ -180,15 +180,35 @@ TYPED_TEST(FloatEnumStoreTest, numbers_can_be_inserted_and_retrieved) } } +TEST(EnumStoreTest, default_value_is_present) +{ + StringEnumStore ses(false, DictionaryConfig::Type::BTREE); + using EntryType = StringEnumStore::EntryType; + EntryType undefined = attribute::getUndefined<EntryType>(); + EnumIndex idx; + EXPECT_TRUE(ses.find_index(undefined, idx)); + EXPECT_TRUE(idx.valid()); + EXPECT_EQ(ses.get_default_value_ref().load_relaxed(), idx); + ses.clear_default_value_ref(); + EXPECT_FALSE(ses.find_index(undefined, idx)); + EXPECT_FALSE(ses.get_default_value_ref().load_relaxed().valid()); + ses.setup_default_value_ref(); + idx = EnumIndex(); + EXPECT_TRUE(ses.find_index(undefined, idx)); + EXPECT_TRUE(idx.valid()); + EXPECT_EQ(ses.get_default_value_ref().load_relaxed(), idx); +} + TEST(EnumStoreTest, test_find_folded_on_string_enum_store) { StringEnumStore ses(false, DictionaryConfig::Type::BTREE); + using EntryType = StringEnumStore::EntryType; std::vector<EnumIndex> indices; std::vector<std::string> unique({"", "one", "two", "TWO", "Two", "three"}); for (std::string &str : unique) { EnumIndex idx = ses.insert(str.c_str()); indices.push_back(idx); - EXPECT_EQ(1u, ses.get_ref_count(idx)); + EXPECT_EQ((str == attribute::getUndefined<EntryType>()) ? 2u : 1u, ses.get_ref_count(idx)); } ses.freeze_dictionary(); for (uint32_t i = 0; i < indices.size(); ++i) { @@ -233,13 +253,14 @@ void StringEnumStoreTest::testInsert(bool hasPostings) { StringEnumStore ses(hasPostings, DictionaryConfig::Type::BTREE); + using EntryType = StringEnumStore::EntryType; std::vector<EnumIndex> indices; std::vector<std::string> unique = {"", "add", "enumstore", "unique"}; for (const auto & i : unique) { EnumIndex idx = ses.insert(i.c_str()); - EXPECT_EQ(1u, ses.get_ref_count(idx)); + EXPECT_EQ((i == attribute::getUndefined<EntryType>()) ? 2u : 1u, ses.get_ref_count(idx)); indices.push_back(idx); EXPECT_TRUE(ses.find_index(i.c_str(), idx)); } @@ -253,7 +274,7 @@ StringEnumStoreTest::testInsert(bool hasPostings) EnumIndex idx; EXPECT_TRUE(ses.find_index(unique[i].c_str(), idx)); EXPECT_TRUE(idx == indices[i]); - EXPECT_EQ(1u, ses.get_ref_count(indices[i])); + EXPECT_EQ((i == 0) ? 2u : 1u, ses.get_ref_count(indices[i])); const char* value = nullptr; EXPECT_TRUE(ses.get_value(indices[i], value)); EXPECT_TRUE(strcmp(unique[i].c_str(), value) == 0); @@ -354,22 +375,22 @@ TEST(EnumStoreTest, address_space_usage_is_reported) NumericEnumStore store(false, DictionaryConfig::Type::BTREE); using vespalib::AddressSpace; - EXPECT_EQ(AddressSpace(1, 1, ADDRESS_LIMIT), store.get_values_address_space_usage()); - EnumIndex idx1 = store.insert(10); EXPECT_EQ(AddressSpace(2, 1, ADDRESS_LIMIT), store.get_values_address_space_usage()); - EnumIndex idx2 = store.insert(20); + EnumIndex idx1 = store.insert(10); // Address limit increases because buffer is re-sized. EXPECT_EQ(AddressSpace(3, 1, ADDRESS_LIMIT + 2), store.get_values_address_space_usage()); + EnumIndex idx2 = store.insert(20); + EXPECT_EQ(AddressSpace(4, 1, ADDRESS_LIMIT + 2), store.get_values_address_space_usage()); dec_ref_count(store, idx1); - EXPECT_EQ(AddressSpace(3, 2, ADDRESS_LIMIT + 2), store.get_values_address_space_usage()); + EXPECT_EQ(AddressSpace(4, 2, ADDRESS_LIMIT + 2), store.get_values_address_space_usage()); dec_ref_count(store, idx2); - EXPECT_EQ(AddressSpace(3, 3, ADDRESS_LIMIT + 2), store.get_values_address_space_usage()); + EXPECT_EQ(AddressSpace(4, 3, ADDRESS_LIMIT + 2), store.get_values_address_space_usage()); } TEST(EnumStoreTest, provided_memory_allocator_is_used) { AllocStats stats; - NumericEnumStore ses(false, DictionaryConfig::Type::BTREE, std::make_unique<MemoryAllocatorObserver>(stats)); + NumericEnumStore ses(false, DictionaryConfig::Type::BTREE, std::make_unique<MemoryAllocatorObserver>(stats), attribute::getUndefined<NumericEnumStore::EntryType>()); EXPECT_EQ(AllocStats(1, 0), stats); } @@ -539,6 +560,7 @@ TYPED_TEST_SUITE(LoaderTest, LoaderTestTypes); TYPED_TEST(LoaderTest, store_is_instantiated_with_enumerated_loader) { + this->store.clear_default_value_ref(); auto loader = this->store.make_enumerated_loader(); this->load_values(loader); loader.allocate_enums_histogram(); @@ -554,6 +576,7 @@ TYPED_TEST(LoaderTest, store_is_instantiated_with_enumerated_loader) TYPED_TEST(LoaderTest, store_is_instantiated_with_enumerated_postings_loader) { + this->store.clear_default_value_ref(); auto loader = this->store.make_enumerated_postings_loader(); this->load_values(loader); this->set_ref_count(0, 1, loader); @@ -568,6 +591,7 @@ TYPED_TEST(LoaderTest, store_is_instantiated_with_enumerated_postings_loader) TYPED_TEST(LoaderTest, store_is_instantiated_with_non_enumerated_loader) { + this->store.clear_default_value_ref(); auto loader = this->store.make_non_enumerated_loader(); using MyValues = LoaderTestValues<typename TypeParam::EnumStoreType>; loader.insert(MyValues::values[0], 100); @@ -610,6 +634,7 @@ public: void test_normalize_posting_lists(bool use_filter, bool one_filter); void test_foreach_posting_list(bool one_filter); static EntryRef fake_pidx() { return EntryRef(42); } + EnumIndex check_default_value_ref() const noexcept; }; template <typename EnumStoreTypeAndDictionaryType> @@ -775,6 +800,16 @@ EnumStoreDictionaryTest<EnumStoreTypeAndDictionaryType>::test_foreach_posting_li clear_sample_values(large_population); } +template <typename EnumStoreTypeAndDictionaryType> +EnumIndex +EnumStoreDictionaryTest<EnumStoreTypeAndDictionaryType>::check_default_value_ref() const noexcept +{ + EnumIndex default_value_ref = store.get_default_value_ref().load_relaxed(); + EXPECT_TRUE(default_value_ref.valid()); + EXPECT_EQ(attribute::getUndefined<EntryType>(), store.get_value(default_value_ref)); + return default_value_ref; +} + using EnumStoreDictionaryTestTypes = ::testing::Types<BTreeNumericEnumStore, HybridNumericEnumStore, HashNumericEnumStore>; TYPED_TEST_SUITE(EnumStoreDictionaryTest, EnumStoreDictionaryTestTypes); @@ -875,6 +910,7 @@ TYPED_TEST(EnumStoreDictionaryTest, compact_worst_works) updater.commit(); generation_t gen = 3; inc_generation(gen, this->store); + // Compact dictionary auto& dict = this->store.get_dictionary(); if (dict.get_has_btree_dictionary()) { EXPECT_LT(CompactionStrategy::DEAD_BYTES_SLACK, dict.get_btree_memory_usage().deadBytes()); @@ -902,8 +938,31 @@ TYPED_TEST(EnumStoreDictionaryTest, compact_worst_works) if (dict.get_has_hash_dictionary()) { EXPECT_GT(CompactionStrategy::DEAD_BYTES_SLACK, dict.get_hash_memory_usage().deadBytes()); } + auto old_default_value_ref = this->check_default_value_ref(); + // Compact values + EXPECT_LT(CompactionStrategy::DEAD_BYTES_SLACK, this->store.get_values_memory_usage().deadBytes()); + compaction_strategy = CompactionStrategy::make_compact_all_active_buffers_strategy(); + int compact_values_count = 0; + for (uint32_t i = 0; i < 2; ++i) { + this->store.update_stat(compaction_strategy); + auto remapper = this->store.consider_compact_values(compaction_strategy); + if (remapper) { + remapper->done(); + ++compact_values_count; + } else { + break; + } + EXPECT_FALSE(this->store.consider_compact_values(compaction_strategy)); + inc_generation(gen, this->store); + } + EXPECT_EQ(1, compact_values_count); + auto new_default_value_ref = this->check_default_value_ref(); + EXPECT_NE(old_default_value_ref, new_default_value_ref); + EXPECT_GT(CompactionStrategy::DEAD_BYTES_SLACK, this->store.get_values_memory_usage().deadBytes()); + std::vector<int32_t> exp_values; std::vector<int32_t> values; + exp_values.push_back(std::numeric_limits<int32_t>::min()); for (int32_t i = 0; i < 20; ++i) { exp_values.push_back(i); } diff --git a/searchlib/src/vespa/searchcommon/common/undefinedvalues.h b/searchlib/src/vespa/searchcommon/common/undefinedvalues.h index bbe3198a8dc..a080648c054 100644 --- a/searchlib/src/vespa/searchcommon/common/undefinedvalues.h +++ b/searchlib/src/vespa/searchcommon/common/undefinedvalues.h @@ -24,6 +24,10 @@ inline constexpr double getUndefined<double>() { return -std::numeric_limits<double>::quiet_NaN(); } +template <> +inline constexpr const char* getUndefined<const char*>() { + return ""; +} // for all signed integers template <typename T> diff --git a/searchlib/src/vespa/searchlib/attribute/attributevector.cpp b/searchlib/src/vespa/searchlib/attribute/attributevector.cpp index f0c8d9a2191..9110c08099a 100644 --- a/searchlib/src/vespa/searchlib/attribute/attributevector.cpp +++ b/searchlib/src/vespa/searchlib/attribute/attributevector.cpp @@ -353,6 +353,8 @@ AttributeVector::load(vespalib::Executor * executor) { bool loaded = onLoad(executor); if (loaded) { commit(); + incGeneration(); + updateStat(true); } _loaded = loaded; return _loaded; @@ -440,6 +442,16 @@ AttributeVector::addReservedDoc() addDoc(docId); // Reserved assert(docId == 0u); assert(docId < getNumDocs()); + set_reserved_doc_values(); +} + +void +AttributeVector::set_reserved_doc_values() +{ + uint32_t docId = 0; + if (docId >= getNumDocs()) { + return; + } clearDoc(docId); if (hasMultiValue()) { if (isFloatingPointType()) { diff --git a/searchlib/src/vespa/searchlib/attribute/attributevector.h b/searchlib/src/vespa/searchlib/attribute/attributevector.h index e40785911ea..b58060a05bf 100644 --- a/searchlib/src/vespa/searchlib/attribute/attributevector.h +++ b/searchlib/src/vespa/searchlib/attribute/attributevector.h @@ -478,6 +478,10 @@ public: * Add reserved initial document with docId 0 and undefined value. */ void addReservedDoc(); + /** + * set undefined values for reserved document 0. + */ + void set_reserved_doc_values(); bool getEnumeratedSave() const { return _hasEnum; } virtual attribute::IPostingListAttributeBase * getIPostingListAttributeBase(); diff --git a/searchlib/src/vespa/searchlib/attribute/enum_store_loaders.cpp b/searchlib/src/vespa/searchlib/attribute/enum_store_loaders.cpp index eeaa3e9539f..c1345b4f770 100644 --- a/searchlib/src/vespa/searchlib/attribute/enum_store_loaders.cpp +++ b/searchlib/src/vespa/searchlib/attribute/enum_store_loaders.cpp @@ -93,6 +93,7 @@ EnumeratedLoader::build_dictionary() { _store.get_dictionary().build(_indexes); release_enum_indexes(); + _store.setup_default_value_ref(); } EnumeratedPostingsLoader::EnumeratedPostingsLoader(IEnumStore& store) @@ -131,6 +132,13 @@ EnumeratedPostingsLoader::build_dictionary() _store.get_dictionary().build_with_payload(_indexes, _posting_indexes); release_enum_indexes(); EntryRefVector().swap(_posting_indexes); + _store.setup_default_value_ref(); +} + +void +EnumeratedPostingsLoader::build_empty_dictionary() +{ + _store.setup_default_value_ref(); } } diff --git a/searchlib/src/vespa/searchlib/attribute/enum_store_loaders.h b/searchlib/src/vespa/searchlib/attribute/enum_store_loaders.h index 2a72fcac628..937ceb91628 100644 --- a/searchlib/src/vespa/searchlib/attribute/enum_store_loaders.h +++ b/searchlib/src/vespa/searchlib/attribute/enum_store_loaders.h @@ -85,6 +85,7 @@ public: void set_ref_count(Index idx, uint32_t ref_count); vespalib::ArrayRef<EntryRef> initialize_empty_posting_indexes(); void build_dictionary(); + void build_empty_dictionary(); }; } diff --git a/searchlib/src/vespa/searchlib/attribute/enumattribute.h b/searchlib/src/vespa/searchlib/attribute/enumattribute.h index f0ff23a06b4..773e5451703 100644 --- a/searchlib/src/vespa/searchlib/attribute/enumattribute.h +++ b/searchlib/src/vespa/searchlib/attribute/enumattribute.h @@ -50,7 +50,7 @@ protected: /* * Iterate through the change vector and find new unique values. - * Perform compaction if necessary and insert the new unique values into the EnumStore. + * Insert the new unique values into the EnumStore. */ void insertNewUniqueValues(EnumStoreBatchUpdater& updater); virtual void considerAttributeChange(const Change & c, EnumStoreBatchUpdater & inserter) = 0; diff --git a/searchlib/src/vespa/searchlib/attribute/enumattribute.hpp b/searchlib/src/vespa/searchlib/attribute/enumattribute.hpp index c5188b89129..f0f518f64f7 100644 --- a/searchlib/src/vespa/searchlib/attribute/enumattribute.hpp +++ b/searchlib/src/vespa/searchlib/attribute/enumattribute.hpp @@ -15,7 +15,7 @@ EnumAttribute<B>:: EnumAttribute(const vespalib::string &baseFileName, const AttributeVector::Config &cfg) : B(baseFileName, cfg), - _enumStore(cfg.fastSearch(), cfg.get_dictionary_config(), this->get_memory_allocator()) + _enumStore(cfg.fastSearch(), cfg.get_dictionary_config(), this->get_memory_allocator(), this->_defaultValue._data.raw()) { this->setEnum(true); } @@ -50,6 +50,7 @@ void EnumAttribute<B>::load_enum_store(LoadedVector& loaded) loader.set_ref_count_for_last_value(prevRefCount); } loader.build_dictionary(); + _enumStore.setup_default_value_ref(); } } @@ -95,5 +96,3 @@ EnumAttribute<B>::cache_change_data_entry_ref(const Change& c) const } } // namespace search - - diff --git a/searchlib/src/vespa/searchlib/attribute/enumstore.h b/searchlib/src/vespa/searchlib/attribute/enumstore.h index 266437fafa1..f6467194d74 100644 --- a/searchlib/src/vespa/searchlib/attribute/enumstore.h +++ b/searchlib/src/vespa/searchlib/attribute/enumstore.h @@ -28,6 +28,9 @@ namespace search { * It uses an instance of vespalib::datastore::UniqueStore to store the actual values. * It also exposes the dictionary used for fast lookups into the set of unique values. * + * The default value is always present except for a short time window + * during attribute vector load. + * * @tparam EntryType The type of the entries/values stored. * It has special handling of type 'const char *' for strings. */ @@ -55,6 +58,8 @@ private: ComparatorType _comparator; ComparatorType _foldedComparator; enumstore::EnumStoreCompactionSpec _compaction_spec; + EntryType _default_value; + AtomicIndex _default_value_ref; EnumStoreT(const EnumStoreT & rhs) = delete; EnumStoreT & operator=(const EnumStoreT & rhs) = delete; @@ -75,7 +80,7 @@ private: std::unique_ptr<EntryComparator> allocate_optionally_folded_comparator(bool folded) const; ComparatorType make_optionally_folded_comparator(bool folded) const; public: - EnumStoreT(bool has_postings, const search::DictionaryConfig& dict_cfg, std::shared_ptr<vespalib::alloc::MemoryAllocator> memory_allocator); + EnumStoreT(bool has_postings, const search::DictionaryConfig& dict_cfg, std::shared_ptr<vespalib::alloc::MemoryAllocator> memory_allocator, EntryType default_value); EnumStoreT(bool has_postings, const search::DictionaryConfig & dict_cfg); ~EnumStoreT() override; @@ -201,6 +206,9 @@ public: bool find_index(EntryType value, Index& idx) const; void free_unused_values() override; void free_unused_values(IndexList to_remove); + void clear_default_value_ref() override; + void setup_default_value_ref() override; + const AtomicIndex& get_default_value_ref() const noexcept { return _default_value_ref; } vespalib::MemoryUsage update_stat(const CompactionStrategy& compaction_strategy) override; std::unique_ptr<EnumIndexRemapper> consider_compact_values(const CompactionStrategy& compaction_strategy) override; std::unique_ptr<EnumIndexRemapper> compact_worst_values(CompactionSpec compaction_spec, const CompactionStrategy& compaction_strategy) override; diff --git a/searchlib/src/vespa/searchlib/attribute/enumstore.hpp b/searchlib/src/vespa/searchlib/attribute/enumstore.hpp index bc767a296eb..c0eebee8e94 100644 --- a/searchlib/src/vespa/searchlib/attribute/enumstore.hpp +++ b/searchlib/src/vespa/searchlib/attribute/enumstore.hpp @@ -17,6 +17,7 @@ #include <vespa/vespalib/datastore/unique_store.hpp> #include <vespa/vespalib/datastore/unique_store_string_allocator.hpp> #include <vespa/vespalib/util/array.hpp> +#include <vespa/searchcommon/common/undefinedvalues.h> #include <vespa/searchlib/util/bufferwriter.h> #include <vespa/vespalib/datastore/compaction_strategy.h> @@ -72,23 +73,26 @@ EnumStoreT<EntryT>::load_unique_value(const void* src, size_t available, Index& } template <typename EntryT> -EnumStoreT<EntryT>::EnumStoreT(bool has_postings, const DictionaryConfig& dict_cfg, std::shared_ptr<vespalib::alloc::MemoryAllocator> memory_allocator) +EnumStoreT<EntryT>::EnumStoreT(bool has_postings, const DictionaryConfig& dict_cfg, std::shared_ptr<vespalib::alloc::MemoryAllocator> memory_allocator, EntryType default_value) : _store(std::move(memory_allocator)), _dict(), _is_folded(dict_cfg.getMatch() == DictionaryConfig::Match::UNCASED), _comparator(_store.get_data_store()), _foldedComparator(make_optionally_folded_comparator(is_folded())), - _compaction_spec() + _compaction_spec(), + _default_value(default_value), + _default_value_ref() { _store.set_dictionary(make_enum_store_dictionary(*this, has_postings, dict_cfg, allocate_comparator(), allocate_optionally_folded_comparator(is_folded()))); _dict = static_cast<IEnumStoreDictionary*>(&_store.get_dictionary()); + setup_default_value_ref(); } template <typename EntryT> EnumStoreT<EntryT>::EnumStoreT(bool has_postings, const DictionaryConfig& dict_cfg) - : EnumStoreT<EntryT>(has_postings, dict_cfg, {}) + : EnumStoreT<EntryT>(has_postings, dict_cfg, {}, attribute::getUndefined<EntryType>()) { } @@ -215,6 +219,33 @@ EnumStoreT<EntryT>::insert(EntryType value) return _store.add(value).ref(); } + +template <typename EntryT> +void +EnumStoreT<EntryT>::clear_default_value_ref() +{ + auto ref = _default_value_ref.load_relaxed(); + if (ref.valid()) { + auto updater = make_batch_updater(); + updater.dec_ref_count(ref); + _default_value_ref.store_relaxed(Index()); + updater.commit(); + } +} + +template <typename EntryT> +void +EnumStoreT<EntryT>::setup_default_value_ref() +{ + if (!_default_value_ref.load_relaxed().valid()) { + auto updater = make_batch_updater(); + auto ref = updater.insert(_default_value); + updater.inc_ref_count(ref); + _default_value_ref.store_relaxed(ref); + updater.commit(); + } +} + template <typename EntryT> vespalib::MemoryUsage EnumStoreT<EntryT>::update_stat(const CompactionStrategy& compaction_strategy) @@ -236,7 +267,14 @@ template <typename EntryT> std::unique_ptr<IEnumStore::EnumIndexRemapper> EnumStoreT<EntryT>::compact_worst_values(CompactionSpec compaction_spec, const CompactionStrategy& compaction_strategy) { - return _store.compact_worst(compaction_spec, compaction_strategy); + auto remapper = _store.compact_worst(compaction_spec, compaction_strategy); + if (remapper) { + auto ref = _default_value_ref.load_relaxed(); + if (ref.valid() && remapper->get_entry_ref_filter().has(ref)) { + _default_value_ref.store_release(remapper->remap(ref)); + } + } + return remapper; } template <typename EntryT> diff --git a/searchlib/src/vespa/searchlib/attribute/i_enum_store.h b/searchlib/src/vespa/searchlib/attribute/i_enum_store.h index 2157db3e5ed..aa9fd549b60 100644 --- a/searchlib/src/vespa/searchlib/attribute/i_enum_store.h +++ b/searchlib/src/vespa/searchlib/attribute/i_enum_store.h @@ -74,6 +74,8 @@ public: virtual std::unique_ptr<Enumerator> make_enumerator() = 0; virtual std::unique_ptr<vespalib::datastore::EntryComparator> allocate_comparator() const = 0; + virtual void clear_default_value_ref() = 0; + virtual void setup_default_value_ref() = 0; }; } diff --git a/searchlib/src/vespa/searchlib/attribute/multinumericenumattribute.hpp b/searchlib/src/vespa/searchlib/attribute/multinumericenumattribute.hpp index edfea23f48d..59c1216829d 100644 --- a/searchlib/src/vespa/searchlib/attribute/multinumericenumattribute.hpp +++ b/searchlib/src/vespa/searchlib/attribute/multinumericenumattribute.hpp @@ -97,6 +97,10 @@ MultiValueNumericEnumAttribute<B, M>::onLoad(vespalib::Executor *) return false; } + this->_enumStore.clear_default_value_ref(); + this->commit(); + this->incGeneration(); + this->setCreateSerialNum(attrReader.getCreateSerialNum()); if (attrReader.getEnumerated()) { diff --git a/searchlib/src/vespa/searchlib/attribute/multistringattribute.hpp b/searchlib/src/vespa/searchlib/attribute/multistringattribute.hpp index a63862126fa..7b11fcd59f4 100644 --- a/searchlib/src/vespa/searchlib/attribute/multistringattribute.hpp +++ b/searchlib/src/vespa/searchlib/attribute/multistringattribute.hpp @@ -42,7 +42,6 @@ MultiValueStringAttributeT<B, M>::freezeEnumDictionary() this->getEnumStore().freeze_dictionary(); } - template <typename B, typename M> std::unique_ptr<attribute::SearchContext> MultiValueStringAttributeT<B, M>::getSearch(QueryTermSimpleUP qTerm, diff --git a/searchlib/src/vespa/searchlib/attribute/postinglistattribute.cpp b/searchlib/src/vespa/searchlib/attribute/postinglistattribute.cpp index 6ef3b575c3e..01e68949f92 100644 --- a/searchlib/src/vespa/searchlib/attribute/postinglistattribute.cpp +++ b/searchlib/src/vespa/searchlib/attribute/postinglistattribute.cpp @@ -49,6 +49,7 @@ PostingListAttributeBase<P>::handle_load_posting_lists_and_update_enum_store(enu PostingChange<P> postings; const auto& loaded_enums = loader.get_loaded_enums(); if (loaded_enums.empty()) { + loader.build_empty_dictionary(); return; } uint32_t preve = 0; diff --git a/searchlib/src/vespa/searchlib/attribute/singlenumericattribute.hpp b/searchlib/src/vespa/searchlib/attribute/singlenumericattribute.hpp index a105d980986..c75ee0aacb5 100644 --- a/searchlib/src/vespa/searchlib/attribute/singlenumericattribute.hpp +++ b/searchlib/src/vespa/searchlib/attribute/singlenumericattribute.hpp @@ -134,8 +134,9 @@ SingleValueNumericAttribute<B>::onLoad(vespalib::Executor *) PrimitiveReader<T> attrReader(*this); bool ok(attrReader.getHasLoadData()); - if (!ok) + if (!ok) { return false; + } this->setCreateSerialNum(attrReader.getCreateSerialNum()); diff --git a/searchlib/src/vespa/searchlib/attribute/singlenumericenumattribute.hpp b/searchlib/src/vespa/searchlib/attribute/singlenumericenumattribute.hpp index 52ea0a53533..14bf9cdc9f0 100644 --- a/searchlib/src/vespa/searchlib/attribute/singlenumericenumattribute.hpp +++ b/searchlib/src/vespa/searchlib/attribute/singlenumericenumattribute.hpp @@ -117,6 +117,10 @@ SingleValueNumericEnumAttribute<B>::onLoad(vespalib::Executor *) return false; } + this->_enumStore.clear_default_value_ref(); + this->commit(); + this->incGeneration(); + this->setCreateSerialNum(attrReader.getCreateSerialNum()); if (attrReader.getEnumerated()) { diff --git a/searchlib/src/vespa/searchlib/attribute/singlestringattribute.hpp b/searchlib/src/vespa/searchlib/attribute/singlestringattribute.hpp index 82a4393fc91..69fe6435a03 100644 --- a/searchlib/src/vespa/searchlib/attribute/singlestringattribute.hpp +++ b/searchlib/src/vespa/searchlib/attribute/singlestringattribute.hpp @@ -40,7 +40,6 @@ SingleValueStringAttributeT<B>::freezeEnumDictionary() this->getEnumStore().freeze_dictionary(); } - template <typename B> std::unique_ptr<attribute::SearchContext> SingleValueStringAttributeT<B>::getSearch(QueryTermSimpleUP qTerm, diff --git a/searchlib/src/vespa/searchlib/attribute/stringbase.cpp b/searchlib/src/vespa/searchlib/attribute/stringbase.cpp index 80967affaa7..b37318d470e 100644 --- a/searchlib/src/vespa/searchlib/attribute/stringbase.cpp +++ b/searchlib/src/vespa/searchlib/attribute/stringbase.cpp @@ -223,6 +223,10 @@ StringAttribute::onLoad(vespalib::Executor *) return false; } + getEnumStoreBase()->clear_default_value_ref(); + commit(); + incGeneration(); + setCreateSerialNum(attrReader.getCreateSerialNum()); assert(attrReader.getEnumerated()); diff --git a/searchlib/src/vespa/searchlib/query/query_term_simple.h b/searchlib/src/vespa/searchlib/query/query_term_simple.h index 74728ab1f2e..a79e33dba32 100644 --- a/searchlib/src/vespa/searchlib/query/query_term_simple.h +++ b/searchlib/src/vespa/searchlib/query/query_term_simple.h @@ -23,7 +23,8 @@ public: SUFFIXTERM = 4, REGEXP = 5, GEO_LOCATION = 6, - FUZZYTERM = 7 + FUZZYTERM = 7, + NEAREST_NEIGHBOR = 8 }; template <typename N> @@ -65,6 +66,7 @@ public: bool isRegex() const { return (_type == Type::REGEXP); } bool isGeoLoc() const { return (_type == Type::GEO_LOCATION); } bool isFuzzy() const { return (_type == Type::FUZZYTERM); } + bool is_nearest_neighbor() const noexcept { return (_type == Type::NEAREST_NEIGHBOR); } bool empty() const { return _term.empty(); } virtual void visitMembers(vespalib::ObjectVisitor &visitor) const; vespalib::string getClassName() const; diff --git a/searchlib/src/vespa/searchlib/query/streaming/CMakeLists.txt b/searchlib/src/vespa/searchlib/query/streaming/CMakeLists.txt index 27f9870dc18..c71b838fb37 100644 --- a/searchlib/src/vespa/searchlib/query/streaming/CMakeLists.txt +++ b/searchlib/src/vespa/searchlib/query/streaming/CMakeLists.txt @@ -1,6 +1,7 @@ # Copyright Yahoo. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. vespa_add_library(searchlib_query_streaming OBJECT SOURCES + nearest_neighbor_query_node.cpp query.cpp querynode.cpp querynoderesultbase.cpp diff --git a/searchlib/src/vespa/searchlib/query/streaming/nearest_neighbor_query_node.cpp b/searchlib/src/vespa/searchlib/query/streaming/nearest_neighbor_query_node.cpp new file mode 100644 index 00000000000..fdc513f9617 --- /dev/null +++ b/searchlib/src/vespa/searchlib/query/streaming/nearest_neighbor_query_node.cpp @@ -0,0 +1,23 @@ +// Copyright Yahoo. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. + +#include "nearest_neighbor_query_node.h" + +namespace search::streaming { + +NearestNeighborQueryNode::NearestNeighborQueryNode(std::unique_ptr<QueryNodeResultBase> resultBase, const string& term, const string& index, int32_t id, search::query::Weight weight, double distance_threshold) + : QueryTerm(std::move(resultBase), term, index, Type::NEAREST_NEIGHBOR), + _distance_threshold(distance_threshold) +{ + setUniqueId(id); + setWeight(weight); +} + +NearestNeighborQueryNode::~NearestNeighborQueryNode() = default; + +NearestNeighborQueryNode* +NearestNeighborQueryNode::as_nearest_neighbor_query_node() noexcept +{ + return this; +} + +} diff --git a/searchlib/src/vespa/searchlib/query/streaming/nearest_neighbor_query_node.h b/searchlib/src/vespa/searchlib/query/streaming/nearest_neighbor_query_node.h new file mode 100644 index 00000000000..ddc84a4b6d3 --- /dev/null +++ b/searchlib/src/vespa/searchlib/query/streaming/nearest_neighbor_query_node.h @@ -0,0 +1,27 @@ +// Copyright Yahoo. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. + +#pragma once + +#include "queryterm.h" + +namespace search::streaming { + +/* + * Nearest neighbor query node. + */ +class NearestNeighborQueryNode: public QueryTerm +{ + double _distance_threshold; +public: + NearestNeighborQueryNode(std::unique_ptr<QueryNodeResultBase> resultBase, const string& term, const string& index, int32_t id, search::query::Weight weight, double distance_threshold); + NearestNeighborQueryNode(const NearestNeighborQueryNode &) = delete; + NearestNeighborQueryNode & operator = (const NearestNeighborQueryNode &) = delete; + NearestNeighborQueryNode(NearestNeighborQueryNode &&) = delete; + NearestNeighborQueryNode & operator = (NearestNeighborQueryNode &&) = delete; + ~NearestNeighborQueryNode() override; + NearestNeighborQueryNode* as_nearest_neighbor_query_node() noexcept override; + const vespalib::string& get_query_tensor_name() const { return getTermString(); } + double get_distance_threshold() const { return _distance_threshold; } +}; + +} diff --git a/searchlib/src/vespa/searchlib/query/streaming/querynode.cpp b/searchlib/src/vespa/searchlib/query/streaming/querynode.cpp index 6d59886a4f5..226cb92c894 100644 --- a/searchlib/src/vespa/searchlib/query/streaming/querynode.cpp +++ b/searchlib/src/vespa/searchlib/query/streaming/querynode.cpp @@ -1,6 +1,7 @@ // Copyright Yahoo. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. #include "query.h" +#include "nearest_neighbor_query_node.h" #include <vespa/searchlib/parsequery/stackdumpiterator.h> #include <charconv> #include <vespa/log/log.h> @@ -77,6 +78,9 @@ QueryNode::Build(const QueryNode * parent, const QueryNodeResultFactory & factor queryRep.getIndexName(), QueryTerm::Type::GEO_LOCATION); break; + case ParseItem::ITEM_NEAREST_NEIGHBOR: + qn = build_nearest_neighbor_query_node(factory, queryRep); + break; case ParseItem::ITEM_NUMTERM: case ParseItem::ITEM_TERM: case ParseItem::ITEM_PREFIXTERM: @@ -191,4 +195,20 @@ const HitList & QueryNode::evaluateHits(HitList & hl) const return hl; } +std::unique_ptr<QueryNode> +QueryNode::build_nearest_neighbor_query_node(const QueryNodeResultFactory& factory, SimpleQueryStackDumpIterator& query_rep) +{ + vespalib::stringref query_tensor_name = query_rep.getTerm(); + vespalib::stringref field_name = query_rep.getIndexName(); + int32_t id = query_rep.getUniqueId(); + search::query::Weight weight = query_rep.GetWeight(); + double distance_threshold = query_rep.getDistanceThreshold(); + return std::make_unique<NearestNeighborQueryNode>(factory.create(), + query_tensor_name, + field_name, + id, + weight, + distance_threshold); +} + } diff --git a/searchlib/src/vespa/searchlib/query/streaming/querynode.h b/searchlib/src/vespa/searchlib/query/streaming/querynode.h index 574a3c16ca3..c3fa2b63f69 100644 --- a/searchlib/src/vespa/searchlib/query/streaming/querynode.h +++ b/searchlib/src/vespa/searchlib/query/streaming/querynode.h @@ -28,6 +28,7 @@ using ConstQueryTermList = std::vector<const QueryTerm *>; */ class QueryNode { + static std::unique_ptr<QueryNode> build_nearest_neighbor_query_node(const QueryNodeResultFactory& factory, SimpleQueryStackDumpIterator& queryRep); public: using UP = std::unique_ptr<QueryNode>; @@ -54,7 +55,7 @@ class QueryNode virtual size_t depth() const { return 1; } /// Return the width of this tree. virtual size_t width() const { return 1; } - static UP Build(const QueryNode * parent, const QueryNodeResultFactory & org, SimpleQueryStackDumpIterator & queryRep, bool allowRewrite); + static UP Build(const QueryNode * parent, const QueryNodeResultFactory& factory, SimpleQueryStackDumpIterator & queryRep, bool allowRewrite); }; /// A list conating the QuerNode objects. With copy/assignment. diff --git a/searchlib/src/vespa/searchlib/query/streaming/queryterm.cpp b/searchlib/src/vespa/searchlib/query/streaming/queryterm.cpp index 83f4410a520..11557bf1dcc 100644 --- a/searchlib/src/vespa/searchlib/query/streaming/queryterm.cpp +++ b/searchlib/src/vespa/searchlib/query/streaming/queryterm.cpp @@ -92,4 +92,10 @@ void QueryTerm::add(unsigned pos, unsigned context, uint32_t elemId, int32_t wei _hitList.emplace_back(pos, context, elemId, weight_); } +NearestNeighborQueryNode* +QueryTerm::as_nearest_neighbor_query_node() noexcept +{ + return nullptr; +} + } diff --git a/searchlib/src/vespa/searchlib/query/streaming/queryterm.h b/searchlib/src/vespa/searchlib/query/streaming/queryterm.h index dd9f56b11e1..51987225692 100644 --- a/searchlib/src/vespa/searchlib/query/streaming/queryterm.h +++ b/searchlib/src/vespa/searchlib/query/streaming/queryterm.h @@ -12,6 +12,8 @@ namespace search::streaming { +class NearestNeighborQueryNode; + /** This is a leaf in the Query tree. All terms are leafs. A QueryTerm has the index for where to find the term. The term is a string, @@ -57,7 +59,7 @@ public: QueryTerm & operator = (const QueryTerm &) = delete; QueryTerm(QueryTerm &&) = delete; QueryTerm & operator = (QueryTerm &&) = delete; - ~QueryTerm(); + ~QueryTerm() override; bool evaluate() const override; const HitList & evaluateHits(HitList & hl) const override; void reset() override; @@ -87,6 +89,7 @@ public: const string & getIndex() const override { return _index; } void setFuzzyMaxEditDistance(uint32_t fuzzyMaxEditDistance) { _fuzzyMaxEditDistance = fuzzyMaxEditDistance; } void setFuzzyPrefixLength(uint32_t fuzzyPrefixLength) { _fuzzyPrefixLength = fuzzyPrefixLength; } + virtual NearestNeighborQueryNode* as_nearest_neighbor_query_node() noexcept; protected: using QueryNodeResultBaseContainer = std::unique_ptr<QueryNodeResultBase>; string _index; diff --git a/streamingvisitors/src/vespa/vsm/config/vsmfields.def b/streamingvisitors/src/vespa/vsm/config/vsmfields.def index 5e943c9274d..a7bd1f03f85 100644 --- a/streamingvisitors/src/vespa/vsm/config/vsmfields.def +++ b/streamingvisitors/src/vespa/vsm/config/vsmfields.def @@ -12,7 +12,7 @@ searchall int default=1 fieldspec[].name string ## The search method for a given field. Note: same field in 2 different document types must match on type if not a random result might be expected. -fieldspec[].searchmethod enum { NONE, BOOL, AUTOUTF8, UTF8, SSE2UTF8, INT8, INT16, INT32, INT64, FLOAT16, FLOAT, DOUBLE, GEOPOS } default=AUTOUTF8 +fieldspec[].searchmethod enum { NONE, BOOL, AUTOUTF8, UTF8, SSE2UTF8, INT8, INT16, INT32, INT64, FLOAT16, FLOAT, DOUBLE, GEOPOS, NEAREST_NEIGHBOR } default=AUTOUTF8 fieldspec[].arg1 string default="" ## Maximum number of chars to search per field. diff --git a/vespa-dependencies-enforcer/allowed-maven-dependencies.txt b/vespa-dependencies-enforcer/allowed-maven-dependencies.txt index 95f1179b1a3..b5841d1c9e4 100644 --- a/vespa-dependencies-enforcer/allowed-maven-dependencies.txt +++ b/vespa-dependencies-enforcer/allowed-maven-dependencies.txt @@ -182,7 +182,7 @@ org.glassfish.jersey.media:jersey-media-multipart:2.25 org.hdrhistogram:HdrHistogram:2.1.12 org.iq80.snappy:snappy:0.4 org.javassist:javassist:3.20.0-GA -org.json:json:20220320 +org.json:json:20230227 org.junit.jupiter:junit-jupiter-api:5.8.1 org.junit.jupiter:junit-jupiter-engine:5.8.1 org.junit.platform:junit-platform-commons:1.8.1 |