diff options
64 files changed, 495 insertions, 739 deletions
diff --git a/client/go/auth/auth0/auth0.go b/client/go/auth/auth0/auth0.go index 52ba3f085a4..9b24066fa04 100644 --- a/client/go/auth/auth0/auth0.go +++ b/client/go/auth/auth0/auth0.go @@ -18,9 +18,7 @@ import ( "time" "github.com/lestrrat-go/jwx/jwt" - "github.com/pkg/browser" "github.com/vespa-engine/vespa/client/go/auth" - "github.com/vespa-engine/vespa/client/go/util" ) const accessTokenExpThreshold = 5 * time.Minute @@ -138,9 +136,7 @@ func (a *Auth0) IsLoggedIn() bool { } // PrepareSystem loads the System, refreshing its token if necessary. -// The System access token needs a refresh if: -// 1. the System scopes are different from the currently required scopes - (auth0 changes). -// 2. the access token is expired. +// The System access token needs a refresh if the access token has expired. func (a *Auth0) PrepareSystem(ctx context.Context) (*System, error) { if err := a.init(); err != nil { return nil, err @@ -150,11 +146,10 @@ func (a *Auth0) PrepareSystem(ctx context.Context) (*System, error) { return nil, err } - if s.AccessToken == "" || scopesChanged(s) { - s, err = RunLogin(ctx, a, true) - if err != nil { - return nil, err - } + if s.AccessToken == "" { + return nil, fmt.Errorf("access token missing: re-authenticate with 'vespa auth login'") + } else if scopesChanged(s) { + return nil, fmt.Errorf("authentication scopes cahnges: re-authenticate with 'vespa auth login'") } else if isExpired(s.ExpiresAt, accessTokenExpThreshold) { // check if the stored access token is expired: // use the refresh token to get a new access token: @@ -257,7 +252,7 @@ func (a *Auth0) AddSystem(s *System) error { return nil } -func (a *Auth0) removeSystem(s string) error { +func (a *Auth0) RemoveSystem(s string) error { _ = a.init() // If we're dealing with an empty file, we'll need to initialize this map. @@ -350,84 +345,3 @@ func (a *Auth0) initContext() (err error) { a.config = *cfg return nil } - -// RunLogin runs the login flow guiding the user through the process -// by showing the login instructions, opening the browser. -// Use `expired` to run the login from other commands setup: -// this will only affect the messages. -func RunLogin(ctx context.Context, a *Auth0, expired bool) (*System, error) { - if expired { - fmt.Println("Please sign in to re-authorize the CLI.") - } - - state, err := a.Authenticator.Start(ctx) - if err != nil { - return nil, fmt.Errorf("could not start the authentication process: %w", err) - } - - fmt.Printf("Your Device Confirmation code is: %s\n\n", state.UserCode) - - fmt.Println("If you prefer, you can open the URL directly for verification") - fmt.Printf("Your Verification URL: %s\n\n", state.VerificationURI) - - fmt.Println("Press Enter to open the browser to log in or ^C to quit...") - fmt.Scanln() - - err = browser.OpenURL(state.VerificationURI) - - if err != nil { - fmt.Printf("Couldn't open the URL, please do it manually: %s.", state.VerificationURI) - } - - var res auth.Result - err = util.Spinner(os.Stderr, "Waiting for login to complete in browser ...", func() error { - res, err = a.Authenticator.Wait(ctx, state) - return err - }) - - if err != nil { - return nil, fmt.Errorf("login error: %w", err) - } - - fmt.Print("\n") - fmt.Println("Successfully logged in.") - fmt.Print("\n") - - // store the refresh token - secretsStore := &auth.Keyring{} - err = secretsStore.Set(auth.SecretsNamespace, a.system, res.RefreshToken) - if err != nil { - // log the error but move on - fmt.Println("Could not store the refresh token locally, please expect to login again once your access token expired.") - } - - s := System{ - Name: a.system, - AccessToken: res.AccessToken, - ExpiresAt: time.Now().Add(time.Duration(res.ExpiresIn) * time.Second), - Scopes: auth.RequiredScopes(), - } - err = a.AddSystem(&s) - if err != nil { - return nil, fmt.Errorf("could not add system to config: %w", err) - } - - return &s, nil -} - -func RunLogout(a *Auth0) error { - s, err := a.getSystem() - if err != nil { - return err - } - - if err := a.removeSystem(s.Name); err != nil { - return err - } - - fmt.Print("\n") - fmt.Println("Successfully logged out.") - fmt.Print("\n") - - return nil -} diff --git a/client/go/cmd/clone.go b/client/go/cmd/clone.go index 180ec18debf..0829e064af9 100644 --- a/client/go/cmd/clone.go +++ b/client/go/cmd/clone.go @@ -19,7 +19,6 @@ import ( "github.com/fatih/color" "github.com/spf13/cobra" - "github.com/vespa-engine/vespa/client/go/util" ) func newCloneCmd(cli *CLI) *cobra.Command { @@ -116,7 +115,7 @@ func fetchSampleAppsZip(destination string, cli *CLI) error { return fmt.Errorf("could not create temporary file: %w", err) } defer f.Close() - return util.Spinner(cli.Stderr, color.YellowString("Downloading sample apps ..."), func() error { + return cli.spinner(cli.Stderr, color.YellowString("Downloading sample apps ..."), func() error { request, err := http.NewRequest("GET", "https://github.com/vespa-engine/sample-apps/archive/refs/heads/master.zip", nil) if err != nil { return fmt.Errorf("invalid url: %w", err) diff --git a/client/go/cmd/deploy.go b/client/go/cmd/deploy.go index cf2176b0c69..fbecd4e19de 100644 --- a/client/go/cmd/deploy.go +++ b/client/go/cmd/deploy.go @@ -12,7 +12,6 @@ import ( "github.com/fatih/color" "github.com/spf13/cobra" - "github.com/vespa-engine/vespa/client/go/util" "github.com/vespa-engine/vespa/client/go/vespa" ) @@ -54,7 +53,7 @@ $ vespa deploy -t cloud -z perf.aws-us-east-1c`, opts := cli.createDeploymentOptions(pkg, target) var result vespa.PrepareResult - err = util.Spinner(cli.Stderr, "Uploading application package ...", func() error { + err = cli.spinner(cli.Stderr, "Uploading application package ...", func() error { result, err = vespa.Deploy(opts) return err }) @@ -103,7 +102,7 @@ func newPrepareCmd(cli *CLI) *cobra.Command { } opts := cli.createDeploymentOptions(pkg, target) var result vespa.PrepareResult - err = util.Spinner(cli.Stderr, "Uploading application package ...", func() error { + err = cli.spinner(cli.Stderr, "Uploading application package ...", func() error { result, err = vespa.Prepare(opts) return err }) diff --git a/client/go/cmd/login.go b/client/go/cmd/login.go index 3750037be88..5cf471ed8db 100644 --- a/client/go/cmd/login.go +++ b/client/go/cmd/login.go @@ -1,10 +1,21 @@ package cmd import ( + "fmt" + "log" + "os" + "time" + + "github.com/pkg/browser" "github.com/spf13/cobra" + "github.com/vespa-engine/vespa/client/go/auth" "github.com/vespa-engine/vespa/client/go/auth/auth0" ) +// newLoginCmd runs the login flow guiding the user through the process +// by showing the login instructions, opening the browser. +// Use `expired` to run the login from other commands setup: +// this will only affect the messages. func newLoginCmd(cli *CLI) *cobra.Command { return &cobra.Command{ Use: "login", @@ -27,7 +38,57 @@ func newLoginCmd(cli *CLI) *cobra.Command { if err != nil { return err } - _, err = auth0.RunLogin(ctx, a, false) + state, err := a.Authenticator.Start(ctx) + if err != nil { + return fmt.Errorf("could not start the authentication process: %w", err) + } + + log.Printf("Your Device Confirmation code is: %s\n\n", state.UserCode) + + log.Println("If you prefer, you can open the URL directly for verification") + log.Printf("Your Verification URL: %s\n\n", state.VerificationURI) + + log.Println("Press Enter to open the browser to log in or ^C to quit...") + fmt.Scanln() + + err = browser.OpenURL(state.VerificationURI) + + if err != nil { + log.Printf("Couldn't open the URL, please do it manually: %s.", state.VerificationURI) + } + + var res auth.Result + err = cli.spinner(os.Stderr, "Waiting for login to complete in browser ...", func() error { + res, err = a.Authenticator.Wait(ctx, state) + return err + }) + + if err != nil { + return fmt.Errorf("login error: %w", err) + } + + log.Print("\n") + log.Println("Successfully logged in.") + log.Print("\n") + + // store the refresh token + secretsStore := &auth.Keyring{} + err = secretsStore.Set(auth.SecretsNamespace, system.Name, res.RefreshToken) + if err != nil { + // log the error but move on + log.Println("Could not store the refresh token locally, please expect to login again once your access token expired.") + } + + s := auth0.System{ + Name: system.Name, + AccessToken: res.AccessToken, + ExpiresAt: time.Now().Add(time.Duration(res.ExpiresIn) * time.Second), + Scopes: auth.RequiredScopes(), + } + err = a.AddSystem(&s) + if err != nil { + return fmt.Errorf("could not add system to config: %w", err) + } return err }, } diff --git a/client/go/cmd/logout.go b/client/go/cmd/logout.go index 6cef5ee371c..c0a6b60ab2e 100644 --- a/client/go/cmd/logout.go +++ b/client/go/cmd/logout.go @@ -1,6 +1,8 @@ package cmd import ( + "log" + "github.com/spf13/cobra" "github.com/vespa-engine/vespa/client/go/auth/auth0" ) @@ -26,8 +28,14 @@ func newLogoutCmd(cli *CLI) *cobra.Command { if err != nil { return err } - err = auth0.RunLogout(a) - return err + if err := a.RemoveSystem(system.Name); err != nil { + return err + } + + log.Print("\n") + log.Println("Successfully logged out.") + log.Print("\n") + return nil }, } } diff --git a/client/go/cmd/root.go b/client/go/cmd/root.go index d352bd2e822..49cab75be53 100644 --- a/client/go/cmd/root.go +++ b/client/go/cmd/root.go @@ -52,6 +52,7 @@ type CLI struct { httpClient util.HTTPClient exec executor isTerminal func() bool + spinner func(w io.Writer, message string, fn func() error) error } // Flags holds the global Flags of Vespa CLI. @@ -141,6 +142,7 @@ Vespa documentation: https://docs.vespa.ai`, if err := cli.loadConfig(); err != nil { return nil, err } + cli.configureSpinner() cli.configureCommands() cmd.PersistentPreRunE = cli.configureOutput return &cli, nil @@ -203,6 +205,19 @@ func (c *CLI) configureFlags() { c.flags = &flags } +func (c *CLI) configureSpinner() { + // Explicitly disable spinner for Screwdriver. It emulates a tty but + // \r result in a newline, and output gets truncated. + _, screwdriver := c.Environment["SCREWDRIVER"] + if c.flags.quiet || !c.isTerminal() || screwdriver { + c.spinner = func(w io.Writer, message string, fn func() error) error { + return fn() + } + } else { + c.spinner = util.Spinner + } +} + func (c *CLI) configureCommands() { rootCmd := c.cmd authCmd := newAuthCmd() diff --git a/client/go/util/spinner.go b/client/go/util/spinner.go index bdce2dbfabe..39b00352c32 100644 --- a/client/go/util/spinner.go +++ b/client/go/util/spinner.go @@ -4,12 +4,10 @@ package util import ( "io" - "os" "strings" "time" "github.com/briandowns/spinner" - "github.com/mattn/go-isatty" ) // Spinner writes message to writer w and executes function fn. While fn is running a spinning animation will be @@ -24,21 +22,11 @@ func Spinner(w io.Writer, message string, fn func() error) error { } s.Prefix = message s.FinalMSG = "\r" + message + "done\n" - isTerminal := isTerminal(w) - if isTerminal { // spinner package does this check too, but it's hardcoded to check os.Stdout :( - s.Start() - } + s.Start() err := fn() - if isTerminal { - s.Stop() - } if err != nil { s.FinalMSG = "\r" + message + "failed\n" } + s.Stop() return err } - -func isTerminal(w io.Writer) bool { - f, ok := w.(*os.File) - return ok && isatty.IsTerminal(f.Fd()) -} diff --git a/clustercontroller-apps/src/main/java/com/yahoo/vespa/clustercontroller/apps/clustercontroller/ClusterController.java b/clustercontroller-apps/src/main/java/com/yahoo/vespa/clustercontroller/apps/clustercontroller/ClusterController.java index 1eca2d18493..5f606c68c1e 100644 --- a/clustercontroller-apps/src/main/java/com/yahoo/vespa/clustercontroller/apps/clustercontroller/ClusterController.java +++ b/clustercontroller-apps/src/main/java/com/yahoo/vespa/clustercontroller/apps/clustercontroller/ClusterController.java @@ -17,6 +17,8 @@ import java.util.LinkedHashMap; import java.util.Map; import java.util.TreeMap; import java.util.concurrent.TimeUnit; +import java.util.concurrent.atomic.AtomicBoolean; +import java.util.concurrent.atomic.AtomicInteger; import java.util.logging.Logger; /** @@ -30,6 +32,8 @@ public class ClusterController extends AbstractComponent private final JDiscMetricWrapper metricWrapper; private final Map<String, FleetController> controllers = new TreeMap<>(); private final Map<String, StatusHandler.ContainerStatusPageServer> status = new TreeMap<>(); + private final AtomicInteger referents = new AtomicInteger(); + private final AtomicBoolean shutdown = new AtomicBoolean(); /** * Dependency injection constructor for controller. A {@link VespaZooKeeperServer} argument is required @@ -43,6 +47,7 @@ public class ClusterController extends AbstractComponent } public void setOptions(FleetControllerOptions options, Metric metricImpl) throws Exception { + referents.incrementAndGet(); metricWrapper.updateMetricImplementation(metricImpl); verifyThatZooKeeperWorks(options); synchronized (controllers) { @@ -60,16 +65,32 @@ public class ClusterController extends AbstractComponent @Override public void deconstruct() { - synchronized (controllers) { - for (FleetController controller : controllers.values()) { - try{ - shutdownController(controller); - } catch (Exception e) { - log.warning("Failed to shut down fleet controller: " + e.getMessage()); + shutdown(); + } + + /** + * Since we hack around injecting a running ZK here by providing one through the configurer instead, + * we must also let the last configurer shut down this controller, to ensure this is shut down + * before the ZK server it had injected from the configurers. + */ + void countdown() { + if (referents.decrementAndGet() == 0) + shutdown(); + } + + void shutdown() { + if (shutdown.compareAndSet(false, true)) { + synchronized (controllers) { + for (FleetController controller : controllers.values()) { + try { + shutdownController(controller); + } + catch (Exception e) { + log.warning("Failed to shut down fleet controller: " + e.getMessage()); + } } } } - super.deconstruct(); } @Override @@ -79,6 +100,8 @@ public class ClusterController extends AbstractComponent } } + FleetController getController(String name) { return controllers.get(name); } + @Override public StatusHandler.ContainerStatusPageServer get(String cluster) { return status.get(cluster); diff --git a/clustercontroller-apps/src/main/java/com/yahoo/vespa/clustercontroller/apps/clustercontroller/ClusterControllerClusterConfigurer.java b/clustercontroller-apps/src/main/java/com/yahoo/vespa/clustercontroller/apps/clustercontroller/ClusterControllerClusterConfigurer.java index 75e838bae3e..a0e290de172 100644 --- a/clustercontroller-apps/src/main/java/com/yahoo/vespa/clustercontroller/apps/clustercontroller/ClusterControllerClusterConfigurer.java +++ b/clustercontroller-apps/src/main/java/com/yahoo/vespa/clustercontroller/apps/clustercontroller/ClusterControllerClusterConfigurer.java @@ -2,6 +2,7 @@ package com.yahoo.vespa.clustercontroller.apps.clustercontroller; import com.google.inject.Inject; +import com.yahoo.component.AbstractComponent; import com.yahoo.jdisc.Metric; import com.yahoo.vdslib.distribution.Distribution; import com.yahoo.vdslib.state.NodeType; @@ -19,9 +20,10 @@ import java.util.Map; * When the cluster controller is reconfigured, a new instance of this is created, which will propagate configured * options to receivers such as the fleet controller. */ -public class ClusterControllerClusterConfigurer { +public class ClusterControllerClusterConfigurer extends AbstractComponent { private final FleetControllerOptions options; + private final ClusterController controller; /** * The {@link VespaZooKeeperServer} argument is required by the injected {@link ClusterController}, @@ -37,9 +39,13 @@ public class ClusterControllerClusterConfigurer { Metric metricImpl, VespaZooKeeperServer started) throws Exception { this.options = configure(distributionConfig, fleetcontrollerConfig, slobroksConfig, zookeepersConfig); - if (controller != null) { - controller.setOptions(options, metricImpl); - } + this.controller = controller; + if (controller != null) controller.setOptions(options, metricImpl); + } + + @Override + public void deconstruct() { + if (controller != null) controller.countdown(); } FleetControllerOptions getOptions() { return options; } diff --git a/clustercontroller-apps/src/test/java/com/yahoo/vespa/clustercontroller/apps/clustercontroller/ClusterControllerClusterConfigurerTest.java b/clustercontroller-apps/src/test/java/com/yahoo/vespa/clustercontroller/apps/clustercontroller/ClusterControllerClusterConfigurerTest.java index f6e64dc7186..dda18fc3396 100644 --- a/clustercontroller-apps/src/test/java/com/yahoo/vespa/clustercontroller/apps/clustercontroller/ClusterControllerClusterConfigurerTest.java +++ b/clustercontroller-apps/src/test/java/com/yahoo/vespa/clustercontroller/apps/clustercontroller/ClusterControllerClusterConfigurerTest.java @@ -11,6 +11,7 @@ import org.junit.Test; import java.util.Map; import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertNotNull; import static org.junit.Assert.assertTrue; import static org.junit.Assert.fail; @@ -50,7 +51,7 @@ public class ClusterControllerClusterConfigurerTest { @Override public Context createContext(Map<String, ?> stringMap) { return null; } }; - // Used in standalone modus to get config without a cluster controller instance + // Used in standalone mode to get config without a cluster controller instance ClusterControllerClusterConfigurer configurer = new ClusterControllerClusterConfigurer( null, new StorDistributionConfig(distributionConfig), @@ -60,7 +61,7 @@ public class ClusterControllerClusterConfigurerTest { metric, null ); - assertTrue(configurer.getOptions() != null); + assertNotNull(configurer.getOptions()); assertEquals(0.123, configurer.getOptions().minNodeRatioPerGroup, 0.01); assertTrue(configurer.getOptions().clusterFeedBlockEnabled); assertEquals(0.5, configurer.getOptions().clusterFeedBlockLimit.get("foo"), 0.01); diff --git a/clustercontroller-apps/src/test/java/com/yahoo/vespa/clustercontroller/apps/clustercontroller/ClusterControllerTest.java b/clustercontroller-apps/src/test/java/com/yahoo/vespa/clustercontroller/apps/clustercontroller/ClusterControllerTest.java index 5c9a7eb9c0c..2c33f781737 100644 --- a/clustercontroller-apps/src/test/java/com/yahoo/vespa/clustercontroller/apps/clustercontroller/ClusterControllerTest.java +++ b/clustercontroller-apps/src/test/java/com/yahoo/vespa/clustercontroller/apps/clustercontroller/ClusterControllerTest.java @@ -11,8 +11,11 @@ import org.junit.Test; import java.util.Map; import java.util.Set; +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertFalse; import static org.junit.Assert.assertNotNull; import static org.junit.Assert.assertNull; +import static org.junit.Assert.assertTrue; /** * Doesn't really test cluster controller, but runs some lines of code. @@ -41,17 +44,16 @@ public class ClusterControllerTest { @Test public void testSimple() throws Exception { - // Cluster controller object keeps state and should never be remade, so should - // inject nothing ClusterController cc = new ClusterController(); cc.setOptions(options, metric); cc.setOptions(options, metric); - cc.getFleetControllers(); - cc.getAll(); - + assertEquals(1, cc.getFleetControllers().size()); assertNotNull(cc.get("storage")); assertNull(cc.get("music")); - cc.deconstruct(); + cc.countdown(); + assertTrue(cc.getController("storage").isRunning()); + cc.countdown(); + assertFalse(cc.getController("storage").isRunning()); } @Test @@ -62,7 +64,7 @@ public class ClusterControllerTest { } }; cc.setOptions(options, metric); - cc.deconstruct(); + cc.countdown(); } } diff --git a/config-model-api/src/main/java/com/yahoo/config/model/api/Model.java b/config-model-api/src/main/java/com/yahoo/config/model/api/Model.java index a7fd48bfea8..54d22d58832 100644 --- a/config-model-api/src/main/java/com/yahoo/config/model/api/Model.java +++ b/config-model-api/src/main/java/com/yahoo/config/model/api/Model.java @@ -70,6 +70,9 @@ public interface Model { /** Returns the version of this model. */ default Version version() { return Version.emptyVersion; } + /** Returns the wanted node version of this model. */ + default Version wantedNodeVersion() { return Version.emptyVersion; } + /** Returns the provisioned hosts of this. */ default Provisioned provisioned() { return new Provisioned(); } diff --git a/config-model/src/main/java/com/yahoo/documentmodel/NewDocumentType.java b/config-model/src/main/java/com/yahoo/documentmodel/NewDocumentType.java index d98869e9dd3..4ff54d7ff1c 100644 --- a/config-model/src/main/java/com/yahoo/documentmodel/NewDocumentType.java +++ b/config-model/src/main/java/com/yahoo/documentmodel/NewDocumentType.java @@ -4,8 +4,10 @@ package com.yahoo.documentmodel; import com.yahoo.document.DataType; import com.yahoo.document.Document; import com.yahoo.document.Field; +import com.yahoo.document.ReferenceDataType; import com.yahoo.document.StructDataType; import com.yahoo.document.StructuredDataType; +import com.yahoo.document.TemporaryStructuredDataType; import com.yahoo.document.annotation.AnnotationType; import com.yahoo.document.annotation.AnnotationTypeRegistry; import com.yahoo.document.datatypes.FieldValue; @@ -383,4 +385,18 @@ public final class NewDocumentType extends StructuredDataType implements DataTyp } + private ReferenceDataType refToThis = null; + + @SuppressWarnings("deprecation") + public ReferenceDataType getReferenceDataType() { + if (refToThis == null) { + // super ugly, the APIs for this are horribly inconsistent + var tmptmp = TemporaryStructuredDataType.create(getName()); + var tmp = ReferenceDataType.createWithInferredId(tmptmp); + tmp.setTargetType((StructuredDataType) this); + refToThis = tmp; + } + return refToThis; + } + } diff --git a/config-model/src/main/java/com/yahoo/searchdefinition/Application.java b/config-model/src/main/java/com/yahoo/searchdefinition/Application.java index 16eef798acd..64688a7e70d 100644 --- a/config-model/src/main/java/com/yahoo/searchdefinition/Application.java +++ b/config-model/src/main/java/com/yahoo/searchdefinition/Application.java @@ -84,7 +84,6 @@ public class Application { List<Schema> schemasSomewhatOrdered = new ArrayList<>(schemas); for (Schema schema : new SearchOrderer().order(schemasSomewhatOrdered)) { - new FieldOperationApplierForStructs().processSchemaFields(schema); new FieldOperationApplierForSearch().process(schema); // TODO: Why is this not in the regular list? new Processing(properties).process(schema, logger, diff --git a/config-model/src/main/java/com/yahoo/searchdefinition/DocumentModelBuilder.java b/config-model/src/main/java/com/yahoo/searchdefinition/DocumentModelBuilder.java index 4a449dc898f..2d9c81085fe 100644 --- a/config-model/src/main/java/com/yahoo/searchdefinition/DocumentModelBuilder.java +++ b/config-model/src/main/java/com/yahoo/searchdefinition/DocumentModelBuilder.java @@ -292,17 +292,8 @@ public class DocumentModelBuilder { else if (type instanceof ReferenceDataType) { ReferenceDataType t = (ReferenceDataType) type; var tt = t.getTargetType(); - if (tt instanceof TemporaryStructuredDataType) { - DataType targetType = resolveTemporariesRecurse(tt, repo, docs, replacements); - t.setTargetType((StructuredDataType) targetType); - } else if (tt instanceof DocumentType) { - DataType targetType = resolveTemporariesRecurse(tt, repo, docs, replacements); - // super ugly, the APIs for this are horribly inconsistent - var tmptmp = TemporaryStructuredDataType.create(tt.getName()); - var tmp = new ReferenceDataType(tmptmp, t.getId()); - tmp.setTargetType((StructuredDataType) targetType); - type = tmp; - } + var doc = getDocumentType(docs, tt.getId()); + type = doc.getReferenceDataType(); } if (type != original) { replacements.add(new TypeReplacement(original, type)); diff --git a/config-model/src/main/java/com/yahoo/searchdefinition/FieldOperationApplierForStructs.java b/config-model/src/main/java/com/yahoo/searchdefinition/FieldOperationApplierForStructs.java index 5e5623e2319..4a5a858f828 100644 --- a/config-model/src/main/java/com/yahoo/searchdefinition/FieldOperationApplierForStructs.java +++ b/config-model/src/main/java/com/yahoo/searchdefinition/FieldOperationApplierForStructs.java @@ -20,47 +20,8 @@ public class FieldOperationApplierForStructs extends FieldOperationApplier { for (SDDocumentType type : sdoc.getAllTypes()) { if (type.isStruct()) { apply(type); - copyFields(type, sdoc); } } } - @SuppressWarnings("deprecation") - private void copyFields(SDDocumentType structType, SDDocumentType sdoc) { - //find all fields in OTHER types that have this type: - List<SDDocumentType> list = new ArrayList<>(); - list.add(sdoc); - list.addAll(sdoc.getTypes()); - for (SDDocumentType anyType : list) { - Iterator<Field> fields = anyType.fieldIterator(); - while (fields.hasNext()) { - SDField field = (SDField) fields.next(); - maybePopulateField(sdoc, field, structType); - } - } - } - - private void maybePopulateField(SDDocumentType sdoc, SDField field, SDDocumentType structType) { - DataType structUsedByField = field.getFirstStructRecursive(); - if (structUsedByField == null) { - return; - } - if (structUsedByField.getName().equals(structType.getName())) { - //this field is using this type!! - field.populateWithStructFields(sdoc, field.getName(), field.getDataType(), 0); - field.populateWithStructMatching(sdoc, field.getDataType(), field.getMatching()); - } - } - - public void processSchemaFields(Schema schema) { - var sdoc = schema.getDocument(); - if (sdoc == null) return; - for (SDDocumentType type : sdoc.getAllTypes()) { - if (type.isStruct()) { - for (SDField field : schema.allExtraFields()) { - maybePopulateField(sdoc, field, type); - } - } - } - } } diff --git a/config-model/src/main/java/com/yahoo/searchdefinition/document/SDDocumentType.java b/config-model/src/main/java/com/yahoo/searchdefinition/document/SDDocumentType.java index a037d5046a7..b87bdd8907e 100644 --- a/config-model/src/main/java/com/yahoo/searchdefinition/document/SDDocumentType.java +++ b/config-model/src/main/java/com/yahoo/searchdefinition/document/SDDocumentType.java @@ -268,8 +268,8 @@ public class SDDocumentType implements Cloneable, Serializable { return field; } - public Field addField(String string, DataType dataType, boolean header, int code) { - SDField field = new SDField(this, string, code, dataType, header); + public Field addField(String fName, DataType dataType, boolean header, int code) { + SDField field = new SDField(this, fName, code, dataType); addField(field); return field; } diff --git a/config-model/src/main/java/com/yahoo/searchdefinition/document/SDField.java b/config-model/src/main/java/com/yahoo/searchdefinition/document/SDField.java index bd6625d4bde..8263352e87f 100644 --- a/config-model/src/main/java/com/yahoo/searchdefinition/document/SDField.java +++ b/config-model/src/main/java/com/yahoo/searchdefinition/document/SDField.java @@ -111,8 +111,8 @@ public class SDField extends Field implements TypedKey, FieldOperationContainer, /** Struct fields defined in this field */ private final Map<String,SDField> structFields = new java.util.LinkedHashMap<>(0); - /** The document that this field was declared in, or null*/ - private SDDocumentType ownerDocType = null; + /** The document that this field was declared in, or null */ + private SDDocumentType repoDocType = null; /** The aliases declared for this field. May pertain to indexes or attributes */ private final Map<String, String> aliasToName = new HashMap<>(); @@ -130,25 +130,24 @@ public class SDField extends Field implements TypedKey, FieldOperationContainer, * @param name the name of the field * @param dataType the datatype of the field */ - protected SDField(SDDocumentType repo, String name, int id, DataType dataType, boolean populate) { + public SDField(SDDocumentType repo, String name, int id, DataType dataType) { super(name, id, dataType); - populate(populate, repo, name, dataType); + this.repoDocType = repo; + populate(name, dataType); } - public SDField(SDDocumentType repo, String name, int id, DataType dataType) { - this(repo, name, id, dataType, true); + public SDField(String name, DataType dataType) { + this(null, name, dataType); } /** Creates a new field */ - public SDField(SDDocumentType repo, String name, DataType dataType, boolean populate) { - super(name, dataType); - populate(populate, repo, name, dataType); + public SDField(SDDocumentType repo, String name, DataType dataType) { + this(repo, name, dataType, null); } /** Creates a new field */ - protected SDField(SDDocumentType repo, String name, DataType dataType, SDDocumentType owner, boolean populate) { - super(name, dataType, owner == null ? null : owner.getDocumentType()); - populate(populate, repo, name, dataType); + protected SDField(SDDocumentType repo, String name, DataType dataType, SDDocumentType owner) { + this(repo, name, dataType, owner, null, 0); } /** @@ -159,27 +158,24 @@ public class SDField extends Field implements TypedKey, FieldOperationContainer, * @param owner the owning document (used to check for id collisions) * @param fieldMatching the matching object to set for the field */ - protected SDField(SDDocumentType repo, String name, DataType dataType, SDDocumentType owner, - Matching fieldMatching, boolean populate, int recursion) { + protected SDField(SDDocumentType repo, + String name, + DataType dataType, + SDDocumentType owner, + Matching fieldMatching, + int recursion) + { super(name, dataType, owner == null ? null : owner.getDocumentType()); + this.repoDocType = repo; + this.structFieldDepth = recursion; if (fieldMatching != null) this.setMatching(fieldMatching); - populate(populate, repo, name, dataType, fieldMatching, recursion); + populate(name, dataType); } - public SDField(SDDocumentType repo, String name, DataType dataType) { - this(repo, name, dataType, true); - } + private int structFieldDepth = 0; - public SDField(String name, DataType dataType) { - this(null, name, dataType); - } - - private void populate(boolean populate, SDDocumentType repo, String name, DataType dataType) { - populate(populate, repo, name, dataType, null, 0); - } - - private void populate(boolean populate, SDDocumentType repo, String name, DataType dataType, Matching fieldMatching, int recursion) { + private void populate(String name, DataType dataType) { if (dataType instanceof TensorDataType) { TensorType type = ((TensorDataType)dataType).getTensorType(); if (type.dimensions().stream().anyMatch(d -> d.isIndexed() && d.size().isEmpty())) @@ -194,10 +190,6 @@ public class SDField extends Field implements TypedKey, FieldOperationContainer, else { addQueryCommand("type " + dataType.getName()); } - if (populate || (dataType instanceof MapDataType)) { - populateWithStructFields(repo, name, dataType, recursion); - populateWithStructMatching(repo, dataType, fieldMatching); - } } public void setIsExtraField(boolean isExtra) { @@ -273,17 +265,23 @@ public class SDField extends Field implements TypedKey, FieldOperationContainer, } } + private boolean doneStructFields = false; + @SuppressWarnings("deprecation") - public void populateWithStructFields(SDDocumentType sdoc, String name, DataType dataType, int recursion) { - DataType dt = getFirstStructOrMapRecursive(); - if (dt == null) return; + private void actuallyMakeStructFields() { + if (doneStructFields) return; + if (getFirstStructOrMapRecursive() == null) { + doneStructFields = true; + return; + } + var sdoc = repoDocType; + var dataType = getDataType(); java.util.function.BiConsumer<String, DataType> supplyStructField = (fieldName, fieldType) -> { if (structFields.containsKey(fieldName)) return; - String subName = name.concat(".").concat(fieldName); - var subField = new SDField(sdoc, subName, fieldType, - ownerDocType, new Matching(), - true, recursion + 1); + String subName = getName().concat(".").concat(fieldName); + var subField = new SDField(sdoc, subName, fieldType, null, + null, structFieldDepth + 1); structFields.put(fieldName, subField); }; @@ -292,15 +290,16 @@ public class SDField extends Field implements TypedKey, FieldOperationContainer, supplyStructField.accept("key", mdt.getKeyType()); supplyStructField.accept("value", mdt.getValueType()); } else { - if (recursion >= 10) return; + if (structFieldDepth >= 10) { + // too risky, infinite recursion + doneStructFields = true; + return; + } if (dataType instanceof CollectionDataType) { dataType = ((CollectionDataType)dataType).getNestedType(); } - if (dataType instanceof TemporaryStructuredDataType) { - SDDocumentType subType = sdoc != null ? sdoc.getType(dataType.getName()) : null; - if (subType == null) { - throw new IllegalArgumentException("Could not find struct '" + dataType.getName() + "'."); - } + SDDocumentType subType = sdoc != null ? sdoc.getType(dataType.getName()) : null; + if (dataType instanceof TemporaryStructuredDataType && subType != null) { for (Field field : subType.fieldSet()) { supplyStructField.accept(field.getName(), field.getDataType()); } @@ -310,37 +309,23 @@ public class SDField extends Field implements TypedKey, FieldOperationContainer, supplyStructField.accept(field.getName(), field.getDataType()); } } - } - } - - public void populateWithStructMatching(SDDocumentType sdoc, DataType dataType, Matching superFieldMatching) { - if (sdoc == null) return; - if (superFieldMatching == null) return; - DataType dt = getFirstStructOrMapRecursive(); - if (dt == null) return; - - if (dataType instanceof MapDataType) { - // old code here would never do anything useful, should we do something here? - return; - } else { - if (dataType instanceof CollectionDataType) { - dataType = ((CollectionDataType)dataType).getNestedType(); + if ((subType == null) && (structFields.size() > 0)) { + throw new IllegalArgumentException("Cannot find matching (repo=" + sdoc + ") for subfields in " + + this + " [" + getDataType() + getDataType().getClass() + + "] with " + structFields.size() + " struct fields"); } - if (dataType instanceof StructDataType) { - SDDocumentType subType = sdoc.getType(dataType.getName()); - if (subType == null) { - throw new IllegalArgumentException("Could not find struct " + dataType.getName()); - } + // populate struct fields with matching + if (subType != null) { for (Field f : subType.fieldSet()) { if (f instanceof SDField) { SDField field = (SDField) f; Matching subFieldMatching = new Matching(); - subFieldMatching.merge(superFieldMatching); + subFieldMatching.merge(this.matching); subFieldMatching.merge(field.getMatching()); SDField subField = structFields.get(field.getName()); if (subField != null) { - subFieldMatching.merge(subField.getMatching()); - subField.populateWithStructMatching(sdoc, field.getDataType(), subFieldMatching); + // we just made this with no matching, so nop: + // subFieldMatching.merge(subField.getMatching()); subField.setMatching(subFieldMatching); } } else { @@ -349,8 +334,11 @@ public class SDField extends Field implements TypedKey, FieldOperationContainer, } } } + doneStructFields = true; } + private Matching matchingForStructFields = null; + public void addOperation(FieldOperation op) { pendingOperations.add(op); } @@ -723,7 +711,10 @@ public class SDField extends Field implements TypedKey, FieldOperationContainer, /** Returns list of static struct fields */ @Override - public Collection<SDField> getStructFields() { return structFields.values(); } + public Collection<SDField> getStructFields() { + actuallyMakeStructFields(); + return structFields.values(); + } /** * Returns a struct field defined in this field, @@ -732,6 +723,7 @@ public class SDField extends Field implements TypedKey, FieldOperationContainer, */ @Override public SDField getStructField(String name) { + actuallyMakeStructFields(); if (name.contains(".")) { String superFieldName = name.substring(0,name.indexOf(".")); String subFieldName = name.substring(name.indexOf(".")+1); diff --git a/config-model/src/main/java/com/yahoo/searchdefinition/document/TemporarySDField.java b/config-model/src/main/java/com/yahoo/searchdefinition/document/TemporarySDField.java index 4ced104fa55..8c17b607f94 100644 --- a/config-model/src/main/java/com/yahoo/searchdefinition/document/TemporarySDField.java +++ b/config-model/src/main/java/com/yahoo/searchdefinition/document/TemporarySDField.java @@ -8,12 +8,12 @@ import com.yahoo.document.DataType; */ public class TemporarySDField extends SDField { - public TemporarySDField(String name, DataType dataType, SDDocumentType owner) { - super(owner, name, dataType, owner, false); + public TemporarySDField(SDDocumentType repo, String name, DataType dataType, SDDocumentType owner) { + super(repo, name, dataType, owner); } - public TemporarySDField(String name, DataType dataType) { - super(null, name, dataType, false); + public TemporarySDField(SDDocumentType repo, String name, DataType dataType) { + super(repo, name, dataType); } } diff --git a/config-model/src/main/java/com/yahoo/searchdefinition/fieldoperation/IndexingOperation.java b/config-model/src/main/java/com/yahoo/searchdefinition/fieldoperation/IndexingOperation.java index fe3ac11af27..a5f5f961ab5 100644 --- a/config-model/src/main/java/com/yahoo/searchdefinition/fieldoperation/IndexingOperation.java +++ b/config-model/src/main/java/com/yahoo/searchdefinition/fieldoperation/IndexingOperation.java @@ -24,6 +24,8 @@ public class IndexingOperation implements FieldOperation { this.script = script; } + public ScriptExpression getScript() { return script; } + public void apply(SDField field) { field.setIndexingScript(script); } diff --git a/config-model/src/main/java/com/yahoo/searchdefinition/parser/ConvertParsedFields.java b/config-model/src/main/java/com/yahoo/searchdefinition/parser/ConvertParsedFields.java index 36c11b33b23..caeebd65f4f 100644 --- a/config-model/src/main/java/com/yahoo/searchdefinition/parser/ConvertParsedFields.java +++ b/config-model/src/main/java/com/yahoo/searchdefinition/parser/ConvertParsedFields.java @@ -291,13 +291,12 @@ public class ConvertParsedFields { schema.addIndex(index); } - SDDocumentType convertStructDeclaration(Schema schema, ParsedStruct parsed) { + SDDocumentType convertStructDeclaration(Schema schema, SDDocumentType document, ParsedStruct parsed) { // TODO - can we cleanup this mess var structProxy = new SDDocumentType(parsed.name(), schema); - structProxy.setStruct(context.resolveStruct(parsed)); for (var parsedField : parsed.getFields()) { var fieldType = context.resolveType(parsedField.getType()); - var field = new SDField(structProxy, parsedField.name(), fieldType); + var field = new SDField(document, parsedField.name(), fieldType); convertCommonFieldSettings(field, parsedField); structProxy.addField(field); if (parsedField.hasIdOverride()) { @@ -307,6 +306,7 @@ public class ConvertParsedFields { for (String inherit : parsed.getInherited()) { structProxy.inherit(new DataTypeName(inherit)); } + structProxy.setStruct(context.resolveStruct(parsed)); return structProxy; } @@ -314,7 +314,7 @@ public class ConvertParsedFields { var annType = context.resolveAnnotation(parsed.name()); var payload = parsed.getStruct(); if (payload.isPresent()) { - var structProxy = convertStructDeclaration(schema, payload.get()); + var structProxy = convertStructDeclaration(schema, document, payload.get()); document.addType(structProxy); } document.addAnnotation(annType); diff --git a/config-model/src/main/java/com/yahoo/searchdefinition/parser/ConvertSchemaCollection.java b/config-model/src/main/java/com/yahoo/searchdefinition/parser/ConvertSchemaCollection.java index 67e6c88d043..2d9a788cfef 100644 --- a/config-model/src/main/java/com/yahoo/searchdefinition/parser/ConvertSchemaCollection.java +++ b/config-model/src/main/java/com/yahoo/searchdefinition/parser/ConvertSchemaCollection.java @@ -159,7 +159,7 @@ public class ConvertSchemaCollection { document.inherit(parent); } for (var struct : parsed.getStructs()) { - var structProxy = fieldConverter.convertStructDeclaration(schema, struct); + var structProxy = fieldConverter.convertStructDeclaration(schema, document, struct); document.addType(structProxy); } for (var annotation : parsed.getAnnotations()) { diff --git a/config-model/src/main/java/com/yahoo/searchdefinition/processing/AddExtraFieldsToDocument.java b/config-model/src/main/java/com/yahoo/searchdefinition/processing/AddExtraFieldsToDocument.java index 51defffa00b..0be48d1fd25 100644 --- a/config-model/src/main/java/com/yahoo/searchdefinition/processing/AddExtraFieldsToDocument.java +++ b/config-model/src/main/java/com/yahoo/searchdefinition/processing/AddExtraFieldsToDocument.java @@ -70,7 +70,7 @@ public class AddExtraFieldsToDocument extends Processor { if (docField == null) { ImmutableSDField existingField = schema.getField(field.getName()); if (existingField == null) { - SDField newField = new SDField(document, field.getName(), field.getDataType(), true); + SDField newField = new SDField(document, field.getName(), field.getDataType()); newField.setIsExtraField(true); document.addField(newField); } else if (!existingField.isImportedField()) { diff --git a/config-model/src/main/java/com/yahoo/searchdefinition/processing/CreatePositionZCurve.java b/config-model/src/main/java/com/yahoo/searchdefinition/processing/CreatePositionZCurve.java index 0bb1b7da769..d7882c7f8fb 100644 --- a/config-model/src/main/java/com/yahoo/searchdefinition/processing/CreatePositionZCurve.java +++ b/config-model/src/main/java/com/yahoo/searchdefinition/processing/CreatePositionZCurve.java @@ -10,6 +10,7 @@ import com.yahoo.document.PositionDataType; import com.yahoo.searchdefinition.Schema; import com.yahoo.searchdefinition.document.Attribute; import com.yahoo.searchdefinition.document.GeoPos; +import com.yahoo.searchdefinition.document.SDDocumentType; import com.yahoo.searchdefinition.document.SDField; import com.yahoo.vespa.documentmodel.SummaryField; import com.yahoo.vespa.documentmodel.SummaryTransform; @@ -35,8 +36,11 @@ import java.util.logging.Level; */ public class CreatePositionZCurve extends Processor { + private final SDDocumentType repo; + public CreatePositionZCurve(Schema schema, DeployLogger deployLogger, RankProfileRegistry rankProfileRegistry, QueryProfiles queryProfiles) { super(schema, deployLogger, rankProfileRegistry, queryProfiles); + this.repo = schema.getDocument(); } private boolean useV8GeoPositions = false; @@ -105,7 +109,7 @@ public class CreatePositionZCurve extends Processor { "' already created."); } boolean isArray = inputField.getDataType() instanceof ArrayDataType; - SDField field = new SDField(fieldName, isArray ? DataType.getArray(DataType.LONG) : DataType.LONG); + SDField field = new SDField(repo, fieldName, isArray ? DataType.getArray(DataType.LONG) : DataType.LONG); Attribute attribute = new Attribute(fieldName, Attribute.Type.LONG, isArray ? Attribute.CollectionType.ARRAY : Attribute.CollectionType.SINGLE); attribute.setPosition(true); diff --git a/config-model/src/main/java/com/yahoo/searchdefinition/processing/UriHack.java b/config-model/src/main/java/com/yahoo/searchdefinition/processing/UriHack.java index 84dc6d369fc..7397f9a289c 100644 --- a/config-model/src/main/java/com/yahoo/searchdefinition/processing/UriHack.java +++ b/config-model/src/main/java/com/yahoo/searchdefinition/processing/UriHack.java @@ -61,7 +61,7 @@ public class UriHack extends Processor { String partName = uriName + "." + suffix; // I wonder if this is explicit in qrs or implicit in backend? // search.addFieldSetItem(uriName, partName); - SDField partField = new SDField(partName, generatedType); + SDField partField = new SDField(schema.getDocument(), partName, generatedType); partField.setIndexStructureField(uriField.doesIndexing()); partField.setRankType(uriField.getRankType()); partField.setStemming(Stemming.NONE); diff --git a/config-model/src/main/java/com/yahoo/vespa/model/VespaModel.java b/config-model/src/main/java/com/yahoo/vespa/model/VespaModel.java index 8390cc59b6f..ac99bee93ed 100644 --- a/config-model/src/main/java/com/yahoo/vespa/model/VespaModel.java +++ b/config-model/src/main/java/com/yahoo/vespa/model/VespaModel.java @@ -108,6 +108,7 @@ public final class VespaModel extends AbstractConfigProducerRoot implements Seri public static final Logger log = Logger.getLogger(VespaModel.class.getName()); private final Version version; + private final Version wantedNodeVersion; private final ConfigModelRepo configModelRepo = new ConfigModelRepo(); private final AllocatedHosts allocatedHosts; @@ -170,6 +171,7 @@ public final class VespaModel extends AbstractConfigProducerRoot implements Seri throws IOException, SAXException { super("vespamodel"); version = deployState.getVespaVersion(); + wantedNodeVersion = deployState.getWantedNodeVespaVersion(); fileReferencesRepository = new FileReferencesRepository(deployState.getFileRegistry()); rankingConstants = new RankingConstants(deployState.getFileRegistry(), Optional.empty()); validationOverrides = deployState.validationOverrides(); @@ -407,6 +409,11 @@ public final class VespaModel extends AbstractConfigProducerRoot implements Seri return version; } + @Override + public Version wantedNodeVersion() { + return wantedNodeVersion; + } + /** * Resolves config of the given type and config id, by first instantiating the correct {@link com.yahoo.config.ConfigInstance.Builder}, * calling {@link #getConfig(com.yahoo.config.ConfigInstance.Builder, String)}. The default values used will be those of the config 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 2742dc59fcd..88139de7888 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 @@ -36,15 +36,13 @@ import java.util.stream.Collectors; */ public class OnnxModelInfo { - private final ApplicationPackage app; private final String modelPath; private final String defaultOutput; private final Map<String, OnnxTypeInfo> inputs; private final Map<String, OnnxTypeInfo> outputs; private final Map<String, TensorType> vespaTypes = new HashMap<>(); - private OnnxModelInfo(ApplicationPackage app, String path, Map<String, OnnxTypeInfo> inputs, Map<String, OnnxTypeInfo> outputs, String defaultOutput) { - this.app = app; + private OnnxModelInfo(String path, Map<String, OnnxTypeInfo> inputs, Map<String, OnnxTypeInfo> outputs, String defaultOutput) { this.modelPath = path; this.inputs = Collections.unmodifiableMap(inputs); this.outputs = Collections.unmodifiableMap(outputs); @@ -81,15 +79,7 @@ public class OnnxModelInfo { Set<Long> unboundSizes = new HashSet<>(); Map<String, Long> symbolicSizes = new HashMap<>(); resolveUnknownDimensionSizes(inputTypes, symbolicSizes, unboundSizes); - - TensorType type = TensorType.empty; - if (inputTypes.size() > 0 && onnxTypeInfo.needModelProbe(symbolicSizes)) { - type = OnnxModelProbe.probeModel(app, Path.fromString(modelPath), onnxName, inputTypes); - } - if (type.equals(TensorType.empty)) { - type = onnxTypeInfo.toVespaTensorType(symbolicSizes, unboundSizes); - } - return type; + return onnxTypeInfo.toVespaTensorType(symbolicSizes, unboundSizes); } return vespaTypes.computeIfAbsent(onnxName, v -> onnxTypeInfo.toVespaTensorType()); } @@ -160,8 +150,7 @@ public class OnnxModelInfo { Onnx.ModelProto model = Onnx.ModelProto.parseFrom(inputStream); String json = onnxModelToJson(model, path); storeGeneratedInfo(json, path, app); - return jsonToModelInfo(json, app); - + return jsonToModelInfo(json); } catch (IOException e) { throw new IllegalArgumentException("Unable to parse ONNX model", e); } @@ -170,7 +159,7 @@ public class OnnxModelInfo { static private OnnxModelInfo loadFromGeneratedInfo(Path path, ApplicationPackage app) { try { String json = readGeneratedInfo(path, app); - return jsonToModelInfo(json, app); + return jsonToModelInfo(json); } catch (IOException e) { throw new IllegalArgumentException("Unable to parse ONNX model", e); } @@ -213,7 +202,7 @@ public class OnnxModelInfo { return out.toString(); } - static public OnnxModelInfo jsonToModelInfo(String json, ApplicationPackage app) throws IOException { + static public OnnxModelInfo jsonToModelInfo(String json) throws IOException { ObjectMapper m = new ObjectMapper(); JsonNode root = m.readTree(json); Map<String, OnnxTypeInfo> inputs = new HashMap<>(); @@ -233,7 +222,7 @@ public class OnnxModelInfo { if (root.get("outputs").has(0)) { defaultOutput = root.get("outputs").get(0).get("name").textValue(); } - return new OnnxModelInfo(app, path, inputs, outputs, defaultOutput); + return new OnnxModelInfo(path, inputs, outputs, defaultOutput); } static private void onnxTypeToJson(JsonGenerator g, Onnx.ValueInfoProto valueInfo) throws IOException { @@ -364,21 +353,6 @@ public class OnnxModelInfo { return builder.build(); } - boolean needModelProbe(Map<String, Long> symbolicSizes) { - for (OnnxDimensionInfo onnxDimension : dimensions) { - if (onnxDimension.hasSymbolicName()) { - if (symbolicSizes == null) - return true; - if ( ! symbolicSizes.containsKey(onnxDimension.getSymbolicName())) { - return true; - } - } else if (onnxDimension.getSize() == 0) { - return true; - } - } - return false; - } - @Override public String toString() { return "(" + valueType.id() + ")" + diff --git a/config-model/src/main/java/com/yahoo/vespa/model/ml/OnnxModelProbe.java b/config-model/src/main/java/com/yahoo/vespa/model/ml/OnnxModelProbe.java deleted file mode 100644 index 2e2ebdeb98f..00000000000 --- a/config-model/src/main/java/com/yahoo/vespa/model/ml/OnnxModelProbe.java +++ /dev/null @@ -1,152 +0,0 @@ -package com.yahoo.vespa.model.ml; - -import com.fasterxml.jackson.core.JsonEncoding; -import com.fasterxml.jackson.core.JsonFactory; -import com.fasterxml.jackson.core.JsonGenerator; -import com.fasterxml.jackson.databind.JsonNode; -import com.fasterxml.jackson.databind.ObjectMapper; -import com.yahoo.config.application.api.ApplicationFile; -import com.yahoo.config.application.api.ApplicationPackage; -import com.yahoo.io.IOUtils; -import com.yahoo.path.Path; -import com.yahoo.tensor.TensorType; - -import java.io.BufferedReader; -import java.io.ByteArrayOutputStream; -import java.io.IOException; -import java.io.InputStream; -import java.io.OutputStream; -import java.nio.charset.StandardCharsets; -import java.util.Map; - -/** - * Defers to 'vespa-analyze-onnx-model' to determine the output type given - * a set of inputs. For situations with symbolic dimension sizes that can't - * easily be determined. - * - * @author lesters - */ -public class OnnxModelProbe { - - private static final String binary = "vespa-analyze-onnx-model"; - - static TensorType probeModel(ApplicationPackage app, Path modelPath, String outputName, Map<String, TensorType> inputTypes) { - TensorType outputType = TensorType.empty; - String contextKey = createContextKey(outputName, inputTypes); - - try { - // Check if output type has already been probed - outputType = readProbedOutputType(app, modelPath, contextKey); - - // Otherwise, run vespa-analyze-onnx-model if the model is available - if (outputType.equals(TensorType.empty) && app.getFile(modelPath).exists()) { - String jsonInput = createJsonInput(app.getFileReference(modelPath).getAbsolutePath(), inputTypes); - String jsonOutput = callVespaAnalyzeOnnxModel(jsonInput); - outputType = outputTypeFromJson(jsonOutput, outputName); - if ( ! outputType.equals(TensorType.empty)) { - writeProbedOutputType(app, modelPath, contextKey, outputType); - } - } - - } catch (IllegalArgumentException | IOException | InterruptedException e) { - e.printStackTrace(System.err); - } - - return outputType; - } - - private static String createContextKey(String onnxName, Map<String, TensorType> inputTypes) { - StringBuilder key = new StringBuilder().append(onnxName).append(":"); - inputTypes.entrySet().stream().sorted(Map.Entry.comparingByKey()) - .forEachOrdered(e -> key.append(e.getKey()).append(":").append(e.getValue()).append(",")); - return key.substring(0, key.length()-1); - } - - private static Path probedOutputTypesPath(Path path) { - String fileName = OnnxModelInfo.asValidIdentifier(path.getRelative()) + ".probed_output_types"; - return ApplicationPackage.MODELS_GENERATED_REPLICATED_DIR.append(fileName); - } - - static void writeProbedOutputType(ApplicationPackage app, Path modelPath, String output, - Map<String, TensorType> inputTypes, TensorType type) throws IOException { - writeProbedOutputType(app, modelPath, createContextKey(output, inputTypes), type); - } - - private static void writeProbedOutputType(ApplicationPackage app, Path modelPath, - String contextKey, TensorType type) throws IOException { - String path = app.getFileReference(probedOutputTypesPath(modelPath)).getAbsolutePath(); - IOUtils.writeFile(path, contextKey + "\t" + type + "\n", true); - } - - private static TensorType readProbedOutputType(ApplicationPackage app, Path modelPath, - String contextKey) throws IOException { - ApplicationFile file = app.getFile(probedOutputTypesPath(modelPath)); - if ( ! file.exists()) { - return TensorType.empty; - } - try (BufferedReader reader = new BufferedReader(file.createReader())) { - String line; - while (null != (line = reader.readLine())) { - String[] parts = line.split("\t"); - String key = parts[0]; - if (key.equals(contextKey)) { - return TensorType.fromSpec(parts[1]); - } - } - } - return TensorType.empty; - } - - private static TensorType outputTypeFromJson(String json, String outputName) throws IOException { - ObjectMapper m = new ObjectMapper(); - JsonNode root = m.readTree(json); - if ( ! root.isObject() || ! root.has("outputs")) { - return TensorType.empty; - } - JsonNode outputs = root.get("outputs"); - if ( ! outputs.has(outputName)) { - return TensorType.empty; - } - return TensorType.fromSpec(outputs.get(outputName).asText()); - } - - private static String createJsonInput(String modelPath, Map<String, TensorType> inputTypes) throws IOException { - ByteArrayOutputStream out = new ByteArrayOutputStream(); - JsonGenerator g = new JsonFactory().createGenerator(out, JsonEncoding.UTF8); - g.writeStartObject(); - g.writeStringField("model", modelPath); - g.writeObjectFieldStart("inputs"); - for (Map.Entry<String, TensorType> input : inputTypes.entrySet()) { - g.writeStringField(input.getKey(), input.getValue().toString()); - } - g.writeEndObject(); - g.writeEndObject(); - g.close(); - return out.toString(); - } - - private static String callVespaAnalyzeOnnxModel(String jsonInput) throws IOException, InterruptedException { - ProcessBuilder processBuilder = new ProcessBuilder(binary, "--probe-types"); - StringBuilder output = new StringBuilder(); - Process process = processBuilder.start(); - - // Write json array to process stdin - OutputStream os = process.getOutputStream(); - os.write(jsonInput.getBytes(StandardCharsets.UTF_8)); - os.close(); - - // Read output from stdout - InputStream inputStream = process.getInputStream(); - while (true) { - int b = inputStream.read(); - if (b == -1) break; - output.append((char)b); - } - int returnCode = process.waitFor(); - if (returnCode != 0) { - throw new IllegalArgumentException("Error from '" + binary + "'. Return code: " + returnCode + ". Output:\n" + output); - } - return output.toString(); - } - -} diff --git a/config-model/src/main/javacc/IntermediateParser.jj b/config-model/src/main/javacc/IntermediateParser.jj index 7447eee5cec..ba955f071b2 100644 --- a/config-model/src/main/javacc/IntermediateParser.jj +++ b/config-model/src/main/javacc/IntermediateParser.jj @@ -958,10 +958,7 @@ void indexingOperation(ParsedField field, boolean multiLine) : { } { { IndexingOperation oldOp = newIndexingOperation(multiLine); - // TODO conversion via SDField is very ugly - SDField tmpField = new SDField("temp", com.yahoo.document.DataType.STRING); - oldOp.apply(tmpField); - ParsedIndexingOp newOp = new ParsedIndexingOp(tmpField.getIndexingScript()); + ParsedIndexingOp newOp = new ParsedIndexingOp(oldOp.getScript()); field.setIndexingOperation(newOp); } } diff --git a/config-model/src/main/javacc/SDParser.jj b/config-model/src/main/javacc/SDParser.jj index 1a0b518bbb2..e90df2776e0 100644 --- a/config-model/src/main/javacc/SDParser.jj +++ b/config-model/src/main/javacc/SDParser.jj @@ -639,7 +639,7 @@ void field(SDDocumentType document, Schema schema) : if (name != null && Schema.isReservedName(name.toLowerCase())) { throw new IllegalArgumentException("Reserved name '" + name + "' can not be used as a field name."); } - field = new TemporarySDField(name, type, document); + field = new TemporarySDField(document == null ? schema.getDocument() : document, name, type, document); } lbrace() (fieldBody(field, schema, document) (<NL>)*)* <RBRACE> { @@ -668,7 +668,7 @@ void fieldSet(Schema schema) : | ( <QUERYCOMMAND> <COLON> (queryCommand = identifierWithDash() | queryCommand = quotedString())) { queryCommands.add(queryCommand); } | - ( matchSetting = match(new SDField(setName, DataType.STRING)) ) { matchSettings.add(matchSetting); } + ( matchSetting = match(new SDField(null, setName, DataType.STRING)) ) { matchSettings.add(matchSetting); } )(<NL>)*)+ <RBRACE> { @@ -698,11 +698,13 @@ void annotationOutside(Schema schema) : <ANNOTATION> name = identifier() { type = new SDAnnotationType(name.trim()); + if (schema.getDocument() == null) { + throw new IllegalArgumentException("Can't add annotation '"+name+"' to a document type, define a document type first or declare the annotation inside of one."); + } } [ inheritsAnnotation(type) (<NL>)* ] - lbrace() (type = annotationBody(schema, type)) <RBRACE> + lbrace() (type = annotationBody(schema, schema.getDocument(), type)) <RBRACE> { - if (schema.getDocument()==null) throw new IllegalArgumentException("Can't add annotation '"+name+"' to a document type, define a document type first or declare the annotation inside of one."); schema.addAnnotation(type); } } @@ -723,7 +725,7 @@ void annotation(Schema schema, SDDocumentType document) : type = new SDAnnotationType(name.trim()); } [ inheritsAnnotation(type) (<NL>)* ] - lbrace() (type = annotationBody(schema, type)) <RBRACE> + lbrace() (type = annotationBody(schema, document, type)) <RBRACE> { document.addAnnotation(type); } @@ -737,12 +739,12 @@ void annotation(Schema schema, SDDocumentType document) : * @param type the type being built * @return a modified or new AnnotationType instance */ -SDAnnotationType annotationBody(Schema schema, SDAnnotationType type) : +SDAnnotationType annotationBody(Schema schema, SDDocumentType repo, SDAnnotationType type) : { SDDocumentType struct = new SDDocumentType("annotation." + type.getName(), schema); } { - (structFieldDefinition(struct) (<NL>)*)* + (structFieldDefinition(repo, struct) (<NL>)*)* { if (struct.getFieldCount() > 0) { // Must account for the temporary TemporarySDField. type = new SDAnnotationType(type.getName(), struct, type.getInherits()); @@ -809,9 +811,13 @@ SDDocumentType structDefinition(Schema schema, SDDocumentType repo) : SDDocumentType struct; } { - ( <STRUCT> name = identifier() (<NL>)* { struct = new SDDocumentType(name, schema); } + ( <STRUCT> name = identifier() (<NL>)* { + if (repo == null) + throw new IllegalArgumentException("Can't add struct '"+name+"' to a document type, define a document type first or declare the struct inside of one."); + struct = new SDDocumentType(name, schema); + } [ inheritsDocument(struct) (<NL>)* ] - lbrace() (structFieldDefinition(struct) (<NL>)*)* <RBRACE> ) + lbrace() (structFieldDefinition(repo, struct) (<NL>)*)* <RBRACE> ) { try { docMan.getDataType(name); @@ -819,7 +825,6 @@ SDDocumentType structDefinition(Schema schema, SDDocumentType repo) : } catch (IllegalArgumentException e) { // empty } - if (repo==null) throw new IllegalArgumentException("Can't add struct '"+name+"' to a document type, define a document type first or declare the struct inside of one."); SDDocumentType sdtype = repo.getOwnedType(struct.getDocumentName()); DataType stype = sdtype != null ? sdtype.getStruct() @@ -828,7 +833,7 @@ SDDocumentType structDefinition(Schema schema, SDDocumentType repo) : return struct; } } - + /** * This rule consumes a data type block from within a field element. * @@ -921,7 +926,7 @@ DataType wildCardType() : * * @param struct The struct to modify. */ -void structFieldDefinition(SDDocumentType struct) : +void structFieldDefinition(SDDocumentType document, SDDocumentType struct) : { String name; SDField field; @@ -932,7 +937,7 @@ void structFieldDefinition(SDDocumentType struct) : if (name != null && Schema.isReservedName(name.toLowerCase())) { throw new IllegalArgumentException("Reserved name '" + name + "' can not be used as a field name."); } - field = new TemporarySDField(name, type, struct); + field = new TemporarySDField(document, name, type, struct); struct.addField(field); } lbrace() (id(field,struct) (<NL>)*)? (match(field) (<NL>)*)* <RBRACE> { diff --git a/config-model/src/test/cfg/application/onnx_probe/files/create_dynamic_model.py b/config-model/src/test/cfg/application/onnx_probe/files/create_dynamic_model.py deleted file mode 100755 index b493e394ee4..00000000000 --- a/config-model/src/test/cfg/application/onnx_probe/files/create_dynamic_model.py +++ /dev/null @@ -1,19 +0,0 @@ -# Copyright Yahoo. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. - -import onnx -import numpy as np -from onnx import helper, TensorProto - -INPUT_1 = helper.make_tensor_value_info('input1', TensorProto.FLOAT, ["batch", 2]) -INPUT_2 = helper.make_tensor_value_info('input2', TensorProto.FLOAT, ["batch", 2]) -OUTPUT = helper.make_tensor_value_info('out', TensorProto.FLOAT, ["batch", "dim1", "dim2"]) - -SHAPE = helper.make_tensor('shape', TensorProto.INT64, dims=[3], vals=np.array([1,2,2]).astype(np.int64)) - -nodes = [ - helper.make_node('Concat', ['input1', 'input2'], ['concat'], axis=1), - helper.make_node('Reshape', ['concat', 'shape'], ['out']), -] -graph_def = helper.make_graph(nodes, 'simple_scoring', [INPUT_1, INPUT_2], [OUTPUT], [SHAPE]) -model_def = helper.make_model(graph_def, producer_name='create_dynamic_model.py', opset_imports=[onnx.OperatorSetIdProto(version=12)]) -onnx.save(model_def, 'dynamic_model_2.onnx') diff --git a/config-model/src/test/cfg/application/onnx_probe/files/dynamic_model.onnx b/config-model/src/test/cfg/application/onnx_probe/files/dynamic_model.onnx deleted file mode 100644 index 28c600e2a09..00000000000 --- a/config-model/src/test/cfg/application/onnx_probe/files/dynamic_model.onnx +++ /dev/null @@ -1,21 +0,0 @@ -create_dynamic_model_2.py:Ö -- -input1 -input2concat"Concat* -axis - -concat -shapeout"Reshapesimple_scoring*:BshapeZ -input1 -
-batch -Z -input2 -
-batch -b& -out - -batch -dim1 -dim2B
\ No newline at end of file diff --git a/config-model/src/test/configmodel/types/references/documentmanager_refs_to_same_type.cfg b/config-model/src/test/configmodel/types/references/documentmanager_refs_to_same_type.cfg index 876ed00d0c4..ef1bb4c5ad4 100644 --- a/config-model/src/test/configmodel/types/references/documentmanager_refs_to_same_type.cfg +++ b/config-model/src/test/configmodel/types/references/documentmanager_refs_to_same_type.cfg @@ -47,8 +47,6 @@ doctype[1].fieldsets{[document]}.fields[0] "campaign_ref" doctype[1].fieldsets{[document]}.fields[1] "other_campaign_ref" doctype[1].documentref[0].idx 10017 doctype[1].documentref[0].targettype 10018 -doctype[1].documentref[1].idx 10019 -doctype[1].documentref[1].targettype 10018 doctype[1].structtype[0].idx 10016 doctype[1].structtype[0].name "ad.header" doctype[1].structtype[0].field[0].name "campaign_ref" @@ -56,10 +54,10 @@ doctype[1].structtype[0].field[0].internalid 23963250 doctype[1].structtype[0].field[0].type 10017 doctype[1].structtype[0].field[1].name "other_campaign_ref" doctype[1].structtype[0].field[1].internalid 874751172 -doctype[1].structtype[0].field[1].type 10019 +doctype[1].structtype[0].field[1].type 10017 doctype[2].name "campaign" doctype[2].idx 10018 doctype[2].inherits[0].idx 10000 -doctype[2].contentstruct 10020 -doctype[2].structtype[0].idx 10020 +doctype[2].contentstruct 10019 +doctype[2].structtype[0].idx 10019 doctype[2].structtype[0].name "campaign.header" diff --git a/config-model/src/test/derived/twostreamingstructs/streamingstruct.sd b/config-model/src/test/derived/twostreamingstructs/streamingstruct.sd index 32823e46592..0636e7a537e 100644 --- a/config-model/src/test/derived/twostreamingstructs/streamingstruct.sd +++ b/config-model/src/test/derived/twostreamingstructs/streamingstruct.sd @@ -21,6 +21,7 @@ search streamingstruct { # Allow default matchtypes in struct. Can be overridden. # No index/attribute related stuff. It is only a datatype definition. } + struct ns1 { field nf1 type s1 { } field nf1s type s1 { match: substring } diff --git a/config-model/src/test/java/com/yahoo/searchdefinition/DocumentGraphValidatorTest.java b/config-model/src/test/java/com/yahoo/searchdefinition/DocumentGraphValidatorTest.java index 81a44261daf..8edbd789a27 100644 --- a/config-model/src/test/java/com/yahoo/searchdefinition/DocumentGraphValidatorTest.java +++ b/config-model/src/test/java/com/yahoo/searchdefinition/DocumentGraphValidatorTest.java @@ -156,8 +156,8 @@ public class DocumentGraphValidatorTest { @SuppressWarnings("deprecation") private static void createDocumentReference(Schema from, Schema to, String refFieldName) { - SDField refField = new TemporarySDField(refFieldName, ReferenceDataType.createWithInferredId(TemporaryStructuredDataType.create(to.getName()))); SDDocumentType fromDocument = from.getDocument(); + SDField refField = new TemporarySDField(fromDocument, refFieldName, ReferenceDataType.createWithInferredId(TemporaryStructuredDataType.create(to.getName()))); fromDocument.addField(refField); Map<String, DocumentReference> originalMap = fromDocument.getDocumentReferences().get().referenceMap(); HashMap<String, DocumentReference> modifiedMap = new HashMap<>(originalMap); diff --git a/config-model/src/test/java/com/yahoo/searchdefinition/DocumentReferenceResolverTest.java b/config-model/src/test/java/com/yahoo/searchdefinition/DocumentReferenceResolverTest.java index 66f1850bd10..4fa145afae9 100644 --- a/config-model/src/test/java/com/yahoo/searchdefinition/DocumentReferenceResolverTest.java +++ b/config-model/src/test/java/com/yahoo/searchdefinition/DocumentReferenceResolverTest.java @@ -38,12 +38,12 @@ public class DocumentReferenceResolverTest { barSchema.addDocument(barDocument); // Create foo document with document reference to bar and add another field - SDField fooRefToBarField = new SDField - ("bar_ref", ReferenceDataType.createWithInferredId(barDocument.getDocumentType())); - AttributeUtils.addAttributeAspect(fooRefToBarField); - SDField irrelevantField = new SDField("irrelevant_stuff", DataType.INT); Schema fooSchema = new Schema(FOO, MockApplicationPackage.createEmpty()); SDDocumentType fooDocument = new SDDocumentType("foo", fooSchema); + SDField fooRefToBarField = new SDField + (fooDocument, "bar_ref", ReferenceDataType.createWithInferredId(barDocument.getDocumentType())); + AttributeUtils.addAttributeAspect(fooRefToBarField); + SDField irrelevantField = new SDField(fooDocument, "irrelevant_stuff", DataType.INT); fooDocument.addField(fooRefToBarField); fooDocument.addField(irrelevantField); fooSchema.addDocument(fooDocument); @@ -62,11 +62,12 @@ public class DocumentReferenceResolverTest { @Test public void throws_user_friendly_exception_if_referenced_document_does_not_exist() { // Create foo document with document reference to non-existing document bar + Schema fooSchema = new Schema(FOO, MockApplicationPackage.createEmpty()); + SDDocumentType fooDocument = new SDDocumentType("foo", fooSchema); SDField fooRefToBarField = new SDField( + fooDocument, "bar_ref", ReferenceDataType.createWithInferredId(TemporaryStructuredDataType.create("bar"))); AttributeUtils.addAttributeAspect(fooRefToBarField); - Schema fooSchema = new Schema(FOO, MockApplicationPackage.createEmpty()); - SDDocumentType fooDocument = new SDDocumentType("foo", fooSchema); fooDocument.addField(fooRefToBarField); fooSchema.addDocument(fooDocument); @@ -86,10 +87,10 @@ public class DocumentReferenceResolverTest { barSchema.addDocument(barDocument); // Create foo document with document reference to bar - SDField fooRefToBarField = new SDField - ("bar_ref", ReferenceDataType.createWithInferredId(barDocument.getDocumentType())); Schema fooSchema = new Schema(FOO, MockApplicationPackage.createEmpty()); SDDocumentType fooDocument = new SDDocumentType("foo", fooSchema); + SDField fooRefToBarField = new SDField + (fooDocument, "bar_ref", ReferenceDataType.createWithInferredId(barDocument.getDocumentType())); fooDocument.addField(fooRefToBarField); fooSchema.addDocument(fooDocument); diff --git a/config-model/src/test/java/com/yahoo/searchdefinition/ImportedFieldsEnumeratorTest.java b/config-model/src/test/java/com/yahoo/searchdefinition/ImportedFieldsEnumeratorTest.java index 7d2386030da..7e708f93a96 100644 --- a/config-model/src/test/java/com/yahoo/searchdefinition/ImportedFieldsEnumeratorTest.java +++ b/config-model/src/test/java/com/yahoo/searchdefinition/ImportedFieldsEnumeratorTest.java @@ -22,16 +22,18 @@ public class ImportedFieldsEnumeratorTest { String PARENT = "parent"; Schema parentSchema = new Schema(PARENT, MockApplicationPackage.createEmpty()); SDDocumentType parentDocument = new SDDocumentType(PARENT, parentSchema); - var parentField = new SDField("their_field", DataType.INT); + var parentField = new SDField(parentDocument, "their_field", DataType.INT); AttributeUtils.addAttributeAspect(parentField); parentDocument.addField(parentField); parentSchema.addDocument(parentDocument); String FOO = "foo"; Schema fooSchema = new Schema(FOO, MockApplicationPackage.createEmpty()); + /* SDField fooRefToParent = new SDField( "foo_ref", ReferenceDataType.createWithInferredId(parentDocument.getDocumentType())); AttributeUtils.addAttributeAspect(fooRefToParent); + */ var fooImports = fooSchema.temporaryImportedFields().get(); fooImports.add(new TemporaryImportedField("my_first_import", "foo_ref", "their_field")); fooImports.add(new TemporaryImportedField("my_second_import", "foo_ref", "their_field")); @@ -40,9 +42,11 @@ public class ImportedFieldsEnumeratorTest { String BAR = "bar"; Schema barSchema = new Schema(BAR, MockApplicationPackage.createEmpty()); + /* SDField barRefToParent = new SDField( "bar_ref", ReferenceDataType.createWithInferredId(parentDocument.getDocumentType())); AttributeUtils.addAttributeAspect(barRefToParent); + */ var barImports = barSchema.temporaryImportedFields().get(); barImports.add(new TemporaryImportedField("my_cool_import", "my_ref", "their_field")); SDDocumentType barDocument = new SDDocumentType(BAR, barSchema); diff --git a/config-model/src/test/java/com/yahoo/searchdefinition/SDDocumentTypeOrdererTestCase.java b/config-model/src/test/java/com/yahoo/searchdefinition/SDDocumentTypeOrdererTestCase.java index c7ec2d9e9d1..652a06a6025 100755 --- a/config-model/src/test/java/com/yahoo/searchdefinition/SDDocumentTypeOrdererTestCase.java +++ b/config-model/src/test/java/com/yahoo/searchdefinition/SDDocumentTypeOrdererTestCase.java @@ -37,22 +37,22 @@ public class SDDocumentTypeOrdererTestCase { g.inherit(new TemporarySDDocumentType(new DataTypeName("e"))); g.inherit(new TemporarySDDocumentType(new DataTypeName("c"))); - SDField aFieldTypeB = new TemporarySDField("atypeb", DataType.STRING); + SDField aFieldTypeB = new TemporarySDField(a, "atypeb", DataType.STRING); a.addField(aFieldTypeB); - SDField bFieldTypeC = new TemporarySDField("btypec", DataType.STRING); + SDField bFieldTypeC = new TemporarySDField(b, "btypec", DataType.STRING); b.addField(bFieldTypeC); - SDField cFieldTypeG = new TemporarySDField("ctypeg", DataType.STRING); + SDField cFieldTypeG = new TemporarySDField(c, "ctypeg", DataType.STRING); c.addField(cFieldTypeG); - SDField gFieldTypeF = new TemporarySDField("gtypef", DataType.STRING); + SDField gFieldTypeF = new TemporarySDField(g, "gtypef", DataType.STRING); g.addField(gFieldTypeF); - SDField fFieldTypeC = new TemporarySDField("ftypec", DataType.STRING); + SDField fFieldTypeC = new TemporarySDField(f, "ftypec", DataType.STRING); f.addField(fFieldTypeC); - SDField dFieldTypeE = new TemporarySDField("dtypee", DataType.STRING); + SDField dFieldTypeE = new TemporarySDField(d, "dtypee", DataType.STRING); d.addField(dFieldTypeE); types.add(a); diff --git a/config-model/src/test/java/com/yahoo/searchdefinition/derived/EmptyRankProfileTestCase.java b/config-model/src/test/java/com/yahoo/searchdefinition/derived/EmptyRankProfileTestCase.java index 237e6c8c992..a3123550efa 100644 --- a/config-model/src/test/java/com/yahoo/searchdefinition/derived/EmptyRankProfileTestCase.java +++ b/config-model/src/test/java/com/yahoo/searchdefinition/derived/EmptyRankProfileTestCase.java @@ -25,11 +25,11 @@ public class EmptyRankProfileTestCase extends AbstractSchemaTestCase { RankProfileRegistry rankProfileRegistry = RankProfileRegistry.createRankProfileRegistryWithBuiltinRankProfiles(schema); SDDocumentType doc = new SDDocumentType("test"); schema.addDocument(doc); - doc.addField(new SDField("a", DataType.STRING)); - SDField field = new SDField("b", DataType.STRING); + doc.addField(new SDField(doc, "a", DataType.STRING)); + SDField field = new SDField(doc, "b", DataType.STRING); field.setLiteralBoost(500); doc.addField(field); - doc.addField(new SDField("c", DataType.STRING)); + doc.addField(new SDField(doc, "c", DataType.STRING)); schema = ApplicationBuilder.buildFromRawSchema(schema, rankProfileRegistry, new QueryProfileRegistry()); new DerivedConfiguration(schema, rankProfileRegistry); diff --git a/config-model/src/test/java/com/yahoo/searchdefinition/derived/IdTestCase.java b/config-model/src/test/java/com/yahoo/searchdefinition/derived/IdTestCase.java index 440f067dd00..ba485b7b96b 100644 --- a/config-model/src/test/java/com/yahoo/searchdefinition/derived/IdTestCase.java +++ b/config-model/src/test/java/com/yahoo/searchdefinition/derived/IdTestCase.java @@ -29,7 +29,7 @@ public class IdTestCase extends AbstractExportingTestCase { Schema schema = new Schema("test", MockApplicationPackage.createEmpty()); SDDocumentType document = new SDDocumentType("test"); schema.addDocument(document); - SDField uri = new SDField("URI", DataType.URI); + SDField uri = new SDField(document, "URI", DataType.URI); uri.parseIndexingScript("{ summary | index }"); document.addField(uri); diff --git a/config-model/src/test/java/com/yahoo/searchdefinition/derived/MatchSettingsResolvingTestCase.java b/config-model/src/test/java/com/yahoo/searchdefinition/derived/MatchSettingsResolvingTestCase.java index 63e3df2205f..275c94465ae 100755 --- a/config-model/src/test/java/com/yahoo/searchdefinition/derived/MatchSettingsResolvingTestCase.java +++ b/config-model/src/test/java/com/yahoo/searchdefinition/derived/MatchSettingsResolvingTestCase.java @@ -54,12 +54,8 @@ public class MatchSettingsResolvingTestCase extends AbstractExportingTestCase { @Test public void testMapWithStructSettings() throws IOException, ParseException { - /* - * does not work - * does not pick up settings from struct declaration assertCorrectDeriving("matchsettings_map_wss", new TestProperties().setExperimentalSdParsing(false)); - */ assertCorrectDeriving("matchsettings_map_wss", new TestProperties().setExperimentalSdParsing(true)); } @@ -74,11 +70,8 @@ public class MatchSettingsResolvingTestCase extends AbstractExportingTestCase { @Test public void testMapAfter() throws IOException, ParseException { - /* fails with: - java.lang.IllegalArgumentException: Could not find struct 'elem'. assertCorrectDeriving("matchsettings_map_after", new TestProperties().setExperimentalSdParsing(false)); - */ assertCorrectDeriving("matchsettings_map_after", new TestProperties().setExperimentalSdParsing(true)); } diff --git a/config-model/src/test/java/com/yahoo/searchdefinition/derived/SchemaOrdererTestCase.java b/config-model/src/test/java/com/yahoo/searchdefinition/derived/SchemaOrdererTestCase.java index 34d33a00d9e..2522f8f56e2 100644 --- a/config-model/src/test/java/com/yahoo/searchdefinition/derived/SchemaOrdererTestCase.java +++ b/config-model/src/test/java/com/yahoo/searchdefinition/derived/SchemaOrdererTestCase.java @@ -91,8 +91,8 @@ public class SchemaOrdererTestCase extends AbstractSchemaTestCase { @SuppressWarnings("deprecation") private static void createDocumentReference(Schema from, Schema to, String refFieldName) { - SDField refField = new TemporarySDField(refFieldName, ReferenceDataType.createWithInferredId(TemporaryStructuredDataType.create(to.getName()))); SDDocumentType fromDocument = from.getDocument(); + SDField refField = new TemporarySDField(fromDocument, refFieldName, ReferenceDataType.createWithInferredId(TemporaryStructuredDataType.create(to.getName()))); fromDocument.addField(refField); Map<String, DocumentReference> originalMap = fromDocument.getDocumentReferences().get().referenceMap(); HashMap<String, DocumentReference> modifiedMap = new HashMap<>(originalMap); diff --git a/config-model/src/test/java/com/yahoo/searchdefinition/derived/TypeConversionTestCase.java b/config-model/src/test/java/com/yahoo/searchdefinition/derived/TypeConversionTestCase.java index dbb32e61144..62a79e49146 100644 --- a/config-model/src/test/java/com/yahoo/searchdefinition/derived/TypeConversionTestCase.java +++ b/config-model/src/test/java/com/yahoo/searchdefinition/derived/TypeConversionTestCase.java @@ -30,7 +30,7 @@ public class TypeConversionTestCase extends AbstractSchemaTestCase { RankProfileRegistry rankProfileRegistry = RankProfileRegistry.createRankProfileRegistryWithBuiltinRankProfiles(schema); SDDocumentType document = new SDDocumentType("test"); schema.addDocument(document); - SDField a = new SDField("a", DataType.STRING); + SDField a = new SDField(document, "a", DataType.STRING); a.parseIndexingScript("{ index }"); document.addField(a); diff --git a/config-model/src/test/java/com/yahoo/searchdefinition/derived/VsmFieldsTestCase.java b/config-model/src/test/java/com/yahoo/searchdefinition/derived/VsmFieldsTestCase.java index 5ab5a8057e8..9c974225605 100644 --- a/config-model/src/test/java/com/yahoo/searchdefinition/derived/VsmFieldsTestCase.java +++ b/config-model/src/test/java/com/yahoo/searchdefinition/derived/VsmFieldsTestCase.java @@ -25,8 +25,9 @@ public class VsmFieldsTestCase { @Test public void reference_type_field_is_unsearchable() { Schema schema = new Schema("test", MockApplicationPackage.createEmpty(), new MockFileRegistry(), new TestableDeployLogger(), new TestProperties()); - schema.addDocument(new SDDocumentType("test")); - SDField refField = new TemporarySDField("ref_field", ReferenceDataType.createWithInferredId(TemporaryStructuredDataType.create("parent_type"))); + var sdoc = new SDDocumentType("test"); + schema.addDocument(sdoc); + SDField refField = new TemporarySDField(sdoc, "ref_field", ReferenceDataType.createWithInferredId(TemporaryStructuredDataType.create("parent_type"))); refField.parseIndexingScript("{ summary }"); schema.getDocument().addField(refField); diff --git a/config-model/src/test/java/com/yahoo/searchdefinition/processing/AddAttributeTransformToSummaryOfImportedFieldsTest.java b/config-model/src/test/java/com/yahoo/searchdefinition/processing/AddAttributeTransformToSummaryOfImportedFieldsTest.java index 9ca97f4dbc7..0adaba4cf68 100644 --- a/config-model/src/test/java/com/yahoo/searchdefinition/processing/AddAttributeTransformToSummaryOfImportedFieldsTest.java +++ b/config-model/src/test/java/com/yahoo/searchdefinition/processing/AddAttributeTransformToSummaryOfImportedFieldsTest.java @@ -59,8 +59,9 @@ public class AddAttributeTransformToSummaryOfImportedFieldsTest { } private static ImportedFields createSingleImportedField(String fieldName) { - Schema targetSchema = createSearch("target_doc"); - SDField targetField = new SDField("target_field", DataType.INT); + Schema targetSchema = createSearchWithDocument("target_doc"); + var doc = targetSchema.getDocument(); + SDField targetField = new SDField(doc, "target_field", DataType.INT); DocumentReference documentReference = new DocumentReference(new Field("reference_field"), targetSchema); ImportedField importedField = new ImportedSimpleField(fieldName, documentReference, targetField); return new ImportedFields(Collections.singletonMap(fieldName, importedField)); diff --git a/config-model/src/test/java/com/yahoo/searchdefinition/processing/AdjustPositionSummaryFieldsTestCase.java b/config-model/src/test/java/com/yahoo/searchdefinition/processing/AdjustPositionSummaryFieldsTestCase.java index 5d2590a420d..926e26451d7 100644 --- a/config-model/src/test/java/com/yahoo/searchdefinition/processing/AdjustPositionSummaryFieldsTestCase.java +++ b/config-model/src/test/java/com/yahoo/searchdefinition/processing/AdjustPositionSummaryFieldsTestCase.java @@ -186,9 +186,10 @@ public class AdjustPositionSummaryFieldsTestCase { private void createPositionField(Schema schema, boolean setupPosAttr, boolean setupBadAttr) { String ilScript = setupPosAttr ? "{ summary | attribute }" : "{ summary }"; - schema.getDocument().addField(createField("pos", PositionDataType.INSTANCE, ilScript)); + var doc = schema.getDocument(); + doc.addField(createField(doc, "pos", PositionDataType.INSTANCE, ilScript)); if (setupBadAttr) { - schema.getDocument().addField(createField("pos_zcurve", DataType.LONG, "{ attribute }")); + doc.addField(createField(doc, "pos_zcurve", DataType.LONG, "{ attribute }")); } } diff --git a/config-model/src/test/java/com/yahoo/searchdefinition/processing/ImportedFieldsResolverTestCase.java b/config-model/src/test/java/com/yahoo/searchdefinition/processing/ImportedFieldsResolverTestCase.java index 324010f9e83..d1d3f4489ce 100644 --- a/config-model/src/test/java/com/yahoo/searchdefinition/processing/ImportedFieldsResolverTestCase.java +++ b/config-model/src/test/java/com/yahoo/searchdefinition/processing/ImportedFieldsResolverTestCase.java @@ -110,13 +110,14 @@ public class ImportedFieldsResolverTestCase { public SearchModel() { super(); grandParentSchema = createSearch("grandparent"); - grandParentSchema.getDocument().addField(createField("ancient_field", DataType.INT, "{ attribute }")); - - parentSchema.getDocument().addField(createField("attribute_field", DataType.INT, "{ attribute }")); - parentSchema.getDocument().addField(createField("attribute_and_index", DataType.INT, "{ attribute | index }")); - parentSchema.getDocument().addField(new TemporarySDField("not_attribute", DataType.INT)); - parentSchema.getDocument().addField(createField("tensor_field", new TensorDataType(TensorType.fromSpec("tensor(x[5])")), "{ attribute }")); - parentSchema.getDocument().addField(createField("predicate_field", DataType.PREDICATE, "{ attribute }")); + var grandParentDoc = grandParentSchema.getDocument(); + grandParentDoc.addField(createField(grandParentDoc, "ancient_field", DataType.INT, "{ attribute }")); + var parentDoc = parentSchema.getDocument(); + parentDoc.addField(createField(parentDoc, "attribute_field", DataType.INT, "{ attribute }")); + parentDoc.addField(createField(parentDoc, "attribute_and_index", DataType.INT, "{ attribute | index }")); + parentDoc.addField(new TemporarySDField(parentDoc, "not_attribute", DataType.INT)); + parentDoc.addField(createField(parentDoc, "tensor_field", new TensorDataType(TensorType.fromSpec("tensor(x[5])")), "{ attribute }")); + parentDoc.addField(createField(parentDoc, "predicate_field", DataType.PREDICATE, "{ attribute }")); addRefField(parentSchema, grandParentSchema, "ref"); addImportedField(parentSchema, "ancient_field", "ref", "ancient_field"); diff --git a/config-model/src/test/java/com/yahoo/searchdefinition/processing/ParentChildSearchModel.java b/config-model/src/test/java/com/yahoo/searchdefinition/processing/ParentChildSearchModel.java index b14c7287537..3b4612ee87a 100644 --- a/config-model/src/test/java/com/yahoo/searchdefinition/processing/ParentChildSearchModel.java +++ b/config-model/src/test/java/com/yahoo/searchdefinition/processing/ParentChildSearchModel.java @@ -37,19 +37,19 @@ public class ParentChildSearchModel { return result; } - protected static TemporarySDField createField(String name, DataType dataType, String indexingScript) { - TemporarySDField result = new TemporarySDField(name, dataType); + protected static TemporarySDField createField(SDDocumentType repo, String name, DataType dataType, String indexingScript) { + TemporarySDField result = new TemporarySDField(repo, name, dataType); result.parseIndexingScript(indexingScript); return result; } @SuppressWarnings("deprecation") - protected static SDField createRefField(String parentType, String fieldName) { - return new TemporarySDField(fieldName, ReferenceDataType.createWithInferredId(TemporaryStructuredDataType.create(parentType))); + protected static SDField createRefField(SDDocumentType repo, String parentType, String fieldName) { + return new TemporarySDField(repo, fieldName, ReferenceDataType.createWithInferredId(TemporaryStructuredDataType.create(parentType))); } protected static void addRefField(Schema child, Schema parent, String fieldName) { - SDField refField = createRefField(parent.getName(), fieldName); + SDField refField = createRefField(child.getDocument(), parent.getName(), fieldName); child.getDocument().addField(refField); child.getDocument().setDocumentReferences(new DocumentReferences(ImmutableMap.of(refField.getName(), new DocumentReference(refField, parent)))); diff --git a/config-model/src/test/java/com/yahoo/searchdefinition/processing/ValidateFieldTypesTest.java b/config-model/src/test/java/com/yahoo/searchdefinition/processing/ValidateFieldTypesTest.java index 22fd4e45c4a..8c801d9deaf 100644 --- a/config-model/src/test/java/com/yahoo/searchdefinition/processing/ValidateFieldTypesTest.java +++ b/config-model/src/test/java/com/yahoo/searchdefinition/processing/ValidateFieldTypesTest.java @@ -65,8 +65,8 @@ public class ValidateFieldTypesTest { } private static ImportedFields createSingleImportedField(String fieldName, DataType dataType) { - Schema targetSchema = createSearch("target_doc"); - SDField targetField = new SDField("target_field", dataType); + Schema targetSchema = createSearchWithDocument("target_doc"); + SDField targetField = new SDField(targetSchema.getDocument(), "target_field", dataType); DocumentReference documentReference = new DocumentReference(new Field("reference_field"), targetSchema); ImportedField importedField = new ImportedSimpleField(fieldName, documentReference, targetField); return new ImportedFields(Collections.singletonMap(fieldName, importedField)); diff --git a/config-model/src/test/java/com/yahoo/vespa/model/ml/OnnxModelProbeTest.java b/config-model/src/test/java/com/yahoo/vespa/model/ml/OnnxModelProbeTest.java deleted file mode 100644 index b2d4953f8e2..00000000000 --- a/config-model/src/test/java/com/yahoo/vespa/model/ml/OnnxModelProbeTest.java +++ /dev/null @@ -1,55 +0,0 @@ -// Copyright Yahoo. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. -package com.yahoo.vespa.model.ml; - -import com.yahoo.config.application.api.ApplicationPackage; -import com.yahoo.config.model.application.provider.FilesApplicationPackage; -import com.yahoo.io.IOUtils; -import com.yahoo.path.Path; -import com.yahoo.tensor.TensorType; -import org.junit.Test; - -import java.io.IOException; -import java.util.Map; - -import static org.junit.Assert.assertEquals; - -public class OnnxModelProbeTest { - - @Test - public void testProbedOutputTypes() throws IOException { - - Path appDir = Path.fromString("src/test/cfg/application/onnx_probe"); - Path storedAppDir = appDir.append("copy"); - try { - FilesApplicationPackage app = FilesApplicationPackage.fromFile(appDir.toFile()); - Path modelPath = Path.fromString("files/dynamic_model.onnx"); - String output = "out"; - Map<String, TensorType> inputTypes = Map.of( - "input1", TensorType.fromSpec("tensor<float>(d0[1],d1[2])"), - "input2", TensorType.fromSpec("tensor<float>(d0[1],d1[2])")); - TensorType expected = TensorType.fromSpec("tensor<float>(d0[1],d1[2],d2[2])"); - - TensorType outputType = OnnxModelProbe.probeModel(app, modelPath, output, inputTypes); - - // if 'vespa-analyze-onnx-model' was unavailable, specifically cache expected type - if (outputType.equals(TensorType.empty)) { - OnnxModelProbe.writeProbedOutputType(app, modelPath, output, inputTypes, expected); - } else { - assertEquals(outputType, expected); - } - - // Test loading from generated info - storedAppDir.toFile().mkdirs(); - IOUtils.copyDirectory(appDir.append(ApplicationPackage.MODELS_GENERATED_DIR).toFile(), - storedAppDir.append(ApplicationPackage.MODELS_GENERATED_DIR).toFile()); - app = FilesApplicationPackage.fromFile(storedAppDir.toFile()); - outputType = OnnxModelProbe.probeModel(app, modelPath, output, inputTypes); - assertEquals(outputType, expected); - - } finally { - IOUtils.recursiveDeleteDir(appDir.append(ApplicationPackage.MODELS_GENERATED_DIR).toFile()); - IOUtils.recursiveDeleteDir(storedAppDir.toFile()); - } - } - -} diff --git a/config-model/src/test/java/com/yahoo/vespa/model/search/test/SchemaClusterTest.java b/config-model/src/test/java/com/yahoo/vespa/model/search/test/SchemaClusterTest.java index e58c29cc4fd..405c1127842 100644 --- a/config-model/src/test/java/com/yahoo/vespa/model/search/test/SchemaClusterTest.java +++ b/config-model/src/test/java/com/yahoo/vespa/model/search/test/SchemaClusterTest.java @@ -42,7 +42,7 @@ public class SchemaClusterTest { // sd1 SDDocumentType sdt1 = new SDDocumentType("s1"); Schema schema1 = new Schema("s1", MockApplicationPackage.createEmpty()); - SDField f1 = new SDField("f1", DataType.STRING); + SDField f1 = new SDField(sdt1, "f1", DataType.STRING); f1.addAttribute(new Attribute("f1", DataType.STRING)); f1.setIndexingScript(new ScriptExpression(new StatementExpression(new AttributeExpression("f1")))); sdt1.addField(f1); @@ -51,7 +51,7 @@ public class SchemaClusterTest { // sd2 SDDocumentType sdt2 = new SDDocumentType("s2"); Schema schema2 = new Schema("s2", MockApplicationPackage.createEmpty()); - SDField f2=new SDField("f2", DataType.STRING); + SDField f2=new SDField(sdt2, "f2", DataType.STRING); f2.addAttribute(new Attribute("f2", DataType.STRING)); f2.setIndexingScript(new ScriptExpression(new StatementExpression(new AttributeExpression("f2")))); sdt2.addField(f2); diff --git a/configserver/src/main/java/com/yahoo/vespa/config/server/application/TenantApplications.java b/configserver/src/main/java/com/yahoo/vespa/config/server/application/TenantApplications.java index 7c669f290d3..5e9e1e8bf33 100644 --- a/configserver/src/main/java/com/yahoo/vespa/config/server/application/TenantApplications.java +++ b/configserver/src/main/java/com/yahoo/vespa/config/server/application/TenantApplications.java @@ -390,10 +390,10 @@ public class TenantApplications implements RequestHandler, HostValidator<Applica @Override public boolean compatibleWith(Optional<Version> vespaVersion, ApplicationId application) { if (vespaVersion.isEmpty()) return true; - Version latestDeployed = applicationMapper.getForVersion(application, Optional.empty(), clock.instant()) - .getVespaVersion(); - boolean compatibleMajor = !incompatibleMajorVersions.value().contains(latestDeployed.getMajor()); - return compatibleMajor || vespaVersion.get().getMajor() == latestDeployed.getMajor(); + Version wantedVersion = applicationMapper.getForVersion(application, Optional.empty(), clock.instant()) + .getModel().wantedNodeVersion(); + boolean compatibleMajor = ! incompatibleMajorVersions.value().contains(wantedVersion.getMajor()); + return compatibleMajor || vespaVersion.get().getMajor() == wantedVersion.getMajor(); } @Override diff --git a/configserver/src/test/java/com/yahoo/vespa/config/server/application/TenantApplicationsTest.java b/configserver/src/test/java/com/yahoo/vespa/config/server/application/TenantApplicationsTest.java index 2b361e825e5..c95c95750a1 100644 --- a/configserver/src/test/java/com/yahoo/vespa/config/server/application/TenantApplicationsTest.java +++ b/configserver/src/test/java/com/yahoo/vespa/config/server/application/TenantApplicationsTest.java @@ -3,10 +3,12 @@ package com.yahoo.vespa.config.server.application; import com.yahoo.cloud.config.ConfigserverConfig; import com.yahoo.component.Version; +import com.yahoo.component.Vtag; import com.yahoo.concurrent.InThreadExecutorService; import com.yahoo.concurrent.StripedExecutor; import com.yahoo.config.model.NullConfigModelRegistry; import com.yahoo.config.model.application.provider.FilesApplicationPackage; +import com.yahoo.config.model.deploy.DeployState; import com.yahoo.config.provision.ApplicationId; import com.yahoo.config.provision.TenantName; import com.yahoo.text.Utf8; @@ -50,6 +52,7 @@ import java.util.Optional; import java.util.Set; import java.util.concurrent.atomic.AtomicInteger; import java.util.function.Function; +import java.util.function.Supplier; import java.util.stream.IntStream; import static com.yahoo.vespa.config.server.application.TenantApplications.RemoveApplicationWaiter; @@ -151,6 +154,19 @@ public class TenantApplicationsTest { assertEquals(0, repo.activeApplications().size()); } + private static ApplicationSet createSet(ApplicationId id, Version version) throws IOException, SAXException { + VespaModel model = new VespaModel(new NullConfigModelRegistry(), + new DeployState.Builder().wantedNodeVespaVersion(version) + .applicationPackage(FilesApplicationPackage.fromFile(new File("src/test/apps/app"))) + .build()); + return ApplicationSet.from(new Application(model, + new ServerCache(), + 1, + Version.emptyVersion, + MetricUpdater.createTestUpdater(), + id)); + } + @Test public void major_version_compatibility() throws Exception { InMemoryFlagSource flagSource = new InMemoryFlagSource(); @@ -158,30 +174,21 @@ public class TenantApplicationsTest { ApplicationId app1 = createApplicationId("myapp"); applications.createApplication(app1); applications.createPutTransaction(app1, 1).commit(); - VespaModel model = new VespaModel(FilesApplicationPackage.fromFile(new File("src/test/apps/app"))); - Function<Version, ApplicationSet> createApplicationSet = (version) -> { - return ApplicationSet.from(new Application(model, - new ServerCache(), - 1, - version, - MetricUpdater.createTestUpdater(), - app1)); - }; Version deployedVersion0 = Version.fromString("6.1"); - applications.activateApplication(createApplicationSet.apply(deployedVersion0), 1); + applications.activateApplication(createSet(app1, deployedVersion0), 1); assertTrue("Empty version is compatible", applications.compatibleWith(Optional.empty(), app1)); Version nodeVersion0 = Version.fromString("6.0"); assertTrue("Lower version is compatible", applications.compatibleWith(Optional.of(nodeVersion0), app1)); Version deployedVersion1 = Version.fromString("7.1"); - applications.activateApplication(createApplicationSet.apply(deployedVersion1), 1); + applications.activateApplication(createSet(app1, deployedVersion1), 1); assertTrue("New major is compatible", applications.compatibleWith(Optional.of(nodeVersion0), app1)); flagSource.withListFlag(PermanentFlags.INCOMPATIBLE_MAJOR_VERSIONS.id(), List.of(8), Integer.class); Version deployedVersion2 = Version.fromString("8.1"); - applications.activateApplication(createApplicationSet.apply(deployedVersion2), 1); + applications.activateApplication(createSet(app1, deployedVersion2), 1); assertFalse("New major is incompatible", applications.compatibleWith(Optional.of(nodeVersion0), app1)); Version nodeVersion1 = Version.fromString("8.0"); diff --git a/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/integration/athenz/AthenzAccessControlService.java b/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/integration/athenz/AthenzAccessControlService.java index 415a087d990..11cace3b10e 100644 --- a/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/integration/athenz/AthenzAccessControlService.java +++ b/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/integration/athenz/AthenzAccessControlService.java @@ -26,11 +26,11 @@ public class AthenzAccessControlService implements AccessControlService { private final ZmsClient zmsClient; private final AthenzRole dataPlaneAccessRole; private final AthenzGroup vespaTeam; - private final ZmsClient vespaZmsClient; //TODO: Merge ZMS clients + private final Optional<ZmsClient> vespaZmsClient; private final AthenzInstanceSynchronizer athenzInstanceSynchronizer; - public AthenzAccessControlService(ZmsClient zmsClient, AthenzDomain domain, ZmsClient vespaZmsClient, AthenzInstanceSynchronizer athenzInstanceSynchronizer) { + public AthenzAccessControlService(ZmsClient zmsClient, AthenzDomain domain, Optional<ZmsClient> vespaZmsClient, AthenzInstanceSynchronizer athenzInstanceSynchronizer) { this.zmsClient = zmsClient; this.vespaZmsClient = vespaZmsClient; this.athenzInstanceSynchronizer = athenzInstanceSynchronizer; @@ -66,11 +66,16 @@ public class AthenzAccessControlService implements AccessControlService { */ @Override public AthenzRoleInformation getAccessRoleInformation(TenantName tenantName) { - var role = sshRole(tenantName); - if (!vespaZmsClient.listRoles(role.domain()).contains(role)) - vespaZmsClient.createRole(role, Map.of()); + return vespaZmsClient.map( + zms -> { + var role = sshRole(tenantName); + if (!zms.listRoles(role.domain()).contains(role)) + zms.createRole(role, Map.of()); + + return zms.getFullRoleInformation(role); + } + ).orElseThrow(() -> new UnsupportedOperationException("Only allowed in systems running Vespa Athenz instance")); - return vespaZmsClient.getFullRoleInformation(role); } /** @@ -78,22 +83,25 @@ public class AthenzAccessControlService implements AccessControlService { */ @Override public boolean decideSshAccess(TenantName tenantName, Instant expiry, OAuthCredentials oAuthCredentials, boolean approve) { - var role = sshRole(tenantName); - - if (!vespaZmsClient.listRoles(role.domain()).contains(role)) - vespaZmsClient.createRole(role, Map.of()); - - if (vespaZmsClient.getMembership(role, vespaTeam)) - return false; - - var roleInformation = vespaZmsClient.getFullRoleInformation(role); - if (roleInformation.getPendingRequest().isEmpty()) - return false; - var reason = roleInformation.getPendingRequest().get().getReason(); - - vespaZmsClient.decidePendingRoleMembership(role, vespaTeam, expiry, Optional.of(reason), Optional.of(oAuthCredentials), approve); - athenzInstanceSynchronizer.synchronizeInstances(tenantName); - return true; + return vespaZmsClient.map( + zms -> { + var role = sshRole(tenantName); + if (!zms.listRoles(role.domain()).contains(role)) + zms.createRole(role, Map.of()); + + if (zms.getMembership(role, vespaTeam)) + return false; + + var roleInformation = zms.getFullRoleInformation(role); + if (roleInformation.getPendingRequest().isEmpty()) + return false; + var reason = roleInformation.getPendingRequest().get().getReason(); + + zms.decidePendingRoleMembership(role, vespaTeam, expiry, Optional.of(reason), Optional.of(oAuthCredentials), approve); + athenzInstanceSynchronizer.synchronizeInstances(tenantName); + return true; + } + ).orElseThrow(() -> new UnsupportedOperationException("Only allowed in systems running Vespa Athenz instance")); } /** @@ -101,40 +109,44 @@ public class AthenzAccessControlService implements AccessControlService { */ @Override public boolean requestSshAccess(TenantName tenantName) { - var role = sshRole(tenantName); + return vespaZmsClient.map( + zms -> { + var role = sshRole(tenantName); - if (!vespaZmsClient.listRoles(role.domain()).contains(role)) - vespaZmsClient.createRole(role, Map.of()); + if (!zms.listRoles(role.domain()).contains(role)) + zms.createRole(role, Map.of()); - if (vespaZmsClient.getMembership(role, vespaTeam)) - return false; + if (zms.getMembership(role, vespaTeam)) + return false; - vespaZmsClient.addRoleMember(role, vespaTeam, Optional.empty()); - return true; + zms.addRoleMember(role, vespaTeam, Optional.empty()); + return true; + } + ).orElseThrow(() -> new UnsupportedOperationException("Only allowed in systems running Vespa Athenz instance")); } public void setPreapprovedAccess(TenantName tenantName, boolean preapprovedAccess) { - var role = sshRole(tenantName); - - var attributes = Map.<String, Object>of( - "selfServe", !preapprovedAccess, - "reviewEnabled", !preapprovedAccess - ); - vespaZmsClient.createRole(role, attributes); + vespaZmsClient.ifPresentOrElse( + zms -> { + var role = sshRole(tenantName); + + var policyName = "vespa-access-requester"; + var action = "update_members"; + var approverRole = new AthenzRole(role.domain(), "vespa-access-approver"); + if (preapprovedAccess) { + zms.addPolicyRule(role.domain(), policyName, action, role.toResourceName(), approverRole); + } else { + zms.deletePolicyRule(role.domain(), policyName, action, role.toResourceName(), approverRole); + } + },() -> { throw new UnsupportedOperationException("Only allowed in systems running Vespa Athenz instance"); }); } private AthenzRole sshRole(TenantName tenantName) { - return new AthenzRole(getOrCreateTenantDomain(tenantName), "ssh_access"); + return new AthenzRole(getTenantDomain(tenantName), "ssh_access"); } - private AthenzDomain getOrCreateTenantDomain(TenantName tenantName) { - var domain = new AthenzDomain(TENANT_DOMAIN_PREFIX + "." + tenantName.value()); - - if (vespaZmsClient.getDomainList(domain.getName()).isEmpty()) { - vespaZmsClient.createSubdomain(new AthenzDomain(TENANT_DOMAIN_PREFIX), tenantName.value()); - } - - return domain; + private AthenzDomain getTenantDomain(TenantName tenantName) { + return new AthenzDomain(TENANT_DOMAIN_PREFIX + "." + tenantName.value()); } public boolean isVespaTeamMember(AthenzUser user) { diff --git a/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/integration/athenz/AthenzDbMock.java b/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/integration/athenz/AthenzDbMock.java index 44271846d7d..63dfff95c03 100644 --- a/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/integration/athenz/AthenzDbMock.java +++ b/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/integration/athenz/AthenzDbMock.java @@ -135,6 +135,19 @@ public class AthenzDbMock { public boolean matches(AthenzIdentity principal, String action, String resource) { return assertions.stream().anyMatch(a -> a.matches(principal, action, resource)); } + + @Override + public boolean equals(Object o) { + if (this == o) return true; + if (o == null || getClass() != o.getClass()) return false; + Policy policy = (Policy) o; + return Objects.equals(name, policy.name) && Objects.equals(assertions, policy.assertions); + } + + @Override + public int hashCode() { + return Objects.hash(name, assertions); + } } public static class Assertion { @@ -192,5 +205,18 @@ public class AthenzDbMock { public String name() { return name; } + + @Override + public boolean equals(Object o) { + if (this == o) return true; + if (o == null || getClass() != o.getClass()) return false; + Role role = (Role) o; + return Objects.equals(name, role.name); + } + + @Override + public int hashCode() { + return Objects.hash(name); + } } } diff --git a/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/integration/organization/DeploymentFailureMails.java b/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/integration/organization/DeploymentFailureMails.java index 2dcb80d28fb..54c4905eb46 100644 --- a/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/integration/organization/DeploymentFailureMails.java +++ b/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/integration/organization/DeploymentFailureMails.java @@ -23,32 +23,35 @@ public class DeploymentFailureMails { public Mail nodeAllocationFailure(RunId id, Collection<String> recipients) { return mail(id, recipients, " due to node allocation failure", "as your node resource request could not be " + - "fulfilled for your tenant. Contact Vespa Cloud support."); + "fulfilled for your tenant. Please contact Vespa Cloud support."); } public Mail deploymentFailure(RunId id, Collection<String> recipients) { return mail(id, recipients, " deployment", "and any previous deployment in the zone is unaffected. " + - "This is usually due to an invalid application configuration, or because " + - "of timeouts waiting for other deployments of the same application to finish."); + "This is usually due to an invalid application configuration. " + + "Please review warnings and errors in the deployment job log."); } public Mail installationFailure(RunId id, Collection<String> recipients) { return mail(id, recipients, "installation", - "as nodes were not able to upgrade to the new configuration. " + + "as nodes were not able to deploy to the new configuration. " + "This is often due to a misconfiguration of the components of an " + - "application, where one or more of these can not be instantiated."); + "application, where one or more of these can not be instantiated. " + + "Please check the Vespa log for errors, and contact Vespa Cloud " + + "support if unable to resolve these."); } public Mail testFailure(RunId id, Collection<String> recipients) { return mail(id, recipients, "tests", - "as one or more verification tests against the deployment failed."); + "as one or more verification tests against the deployment failed. " + + "Please review test output in the deployment job log."); } public Mail systemError(RunId id, Collection<String> recipients) { return mail(id, recipients, "due to system error", - "as something in the framework went wrong. Such errors are " + - "usually transient. Please contact the Vespa team if the problem persists!"); + "as something in the deployment framework went wrong. Such errors are " + + "usually transient. Please contact Vespa Cloud support if the problem persists."); } private Mail mail(RunId id, Collection<String> recipients, String summaryDetail, String messageDetail) { @@ -72,7 +75,8 @@ public class DeploymentFailureMails { return "System test"; if (type == JobType.stagingTest) return "Staging test"; - return "Deployment to " + type.zone(registry.system()).region(); + return (type.isDeployment() ? "Deployment to " : "Verification test of ") + + type.zone(registry.system()).region(); } } 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 8c7e60bd199..beef090d214 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 @@ -823,16 +823,16 @@ public class InternalStepRunner implements StepRunner { controller.notificationsDb().removeNotification(source, Notification.Type.deployment); return; case nodeAllocationFailure: - if ( ! run.id().type().environment().isTest()) updater.accept("could not allocate the requested capacity to your tenant. Contact Vespa Cloud support."); + if ( ! run.id().type().environment().isTest()) updater.accept("could not allocate the requested capacity to your tenant. Please contact Vespa Cloud support."); return; case deploymentFailed: - updater.accept("invalid application configuration, or timeout of other deployments of the same application"); + updater.accept("invalid application configuration. Please review warnings and errors in the deployment job log."); return; case installationFailed: - updater.accept("nodes were not able to upgrade to the new configuration"); + updater.accept("nodes were not able to deploy to the new configuration. Please check the Vespa log for errors, and contact Vespa Cloud support if unable to resolve these."); return; case testFailure: - updater.accept("one or more verification tests against the deployment failed"); + updater.accept("one or more verification tests against the deployment failed. Please review test output in the deployment job log."); return; case error: case endpointCertificateTimeout: @@ -840,8 +840,8 @@ public class InternalStepRunner implements StepRunner { default: logger.log(WARNING, "Don't know what to set console notification to for run status '" + run.status() + "'"); } - updater.accept("something in the framework went wrong. Such errors are " + - "usually transient. Please contact the Vespa team if the problem persists!"); + updater.accept("something in the deployment framework went wrong. Such errors are " + + "usually transient. Please contact Vespa Cloud support if the problem persists."); } private Optional<Mail> mailOf(Run run, List<String> recipients) { diff --git a/document/src/main/java/com/yahoo/document/DataType.java b/document/src/main/java/com/yahoo/document/DataType.java index e47c60863ff..2fcbe7333b4 100644 --- a/document/src/main/java/com/yahoo/document/DataType.java +++ b/document/src/main/java/com/yahoo/document/DataType.java @@ -62,6 +62,7 @@ public abstract class DataType extends Identifiable implements Serializable, Com static { TAG.setTag(true); } + public static int lastPredefinedDataTypeId() { return 21; } diff --git a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/provisioning/HostProvisioner.java b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/provisioning/HostProvisioner.java index dc39aa722c7..ce6ac9d5f5f 100644 --- a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/provisioning/HostProvisioner.java +++ b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/provisioning/HostProvisioner.java @@ -23,12 +23,10 @@ public interface HostProvisioner { /** The host must be provisioned exclusively for the applicationId */ exclusive, - /** The host must be provisioned to be shared with other applications. \ - */ + /** The host must be provisioned to be shared with other applications. */ shared, - /** The client has no requirements on whether the host must be provisio\ - ned exclusively or shared. */ + /** The client has no requirements on whether the host must be provisioned exclusively or shared. */ any } diff --git a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/provisioning/ProvisionedHost.java b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/provisioning/ProvisionedHost.java index 7643368aad9..b5fd8c8111f 100644 --- a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/provisioning/ProvisionedHost.java +++ b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/provisioning/ProvisionedHost.java @@ -73,27 +73,17 @@ public class ProvisionedHost { return Node.reserve(Set.of(), nodeHostname(), hostHostname, nodeResources, hostType.childNodeType()).build(); } - public String getId() { - return id; - } - - public String hostHostname() { - return hostHostname; - } - - public Flavor hostFlavor() { - return hostFlavor; - } - - public String nodeHostname() { - return nodeAddresses.get(0).hostname(); - } - - public List<Address> nodeAddresses() { - return nodeAddresses; - } - + public String getId() { return id; } + public String hostHostname() { return hostHostname; } + public Flavor hostFlavor() { return hostFlavor; } + public NodeType hostType() { return hostType; } + public Optional<ApplicationId> exclusiveToApplicationId() { return exclusiveToApplicationId; } + public Optional<ClusterSpec.Type> exclusiveToClusterType() { return exclusiveToClusterType; } + public List<Address> nodeAddresses() { return nodeAddresses; } public NodeResources nodeResources() { return nodeResources; } + public Version osVersion() { return osVersion; } + + public String nodeHostname() { return nodeAddresses.get(0).hostname(); } @Override public boolean equals(Object o) { diff --git a/parent/pom.xml b/parent/pom.xml index cac01fc6d5e..c16f812f80c 100644 --- a/parent/pom.xml +++ b/parent/pom.xml @@ -75,9 +75,6 @@ <artifactId>maven-compiler-plugin</artifactId> <version>${maven-compiler-plugin.version}</version> <configuration> - <jdkToolchain> - <version>11</version> - </jdkToolchain> <release>11</release> <showWarnings>true</showWarnings> <optimize>true</optimize> diff --git a/searchcore/src/vespa/searchcore/proton/server/storeonlyfeedview.cpp b/searchcore/src/vespa/searchcore/proton/server/storeonlyfeedview.cpp index f34885974b8..a537742b79b 100644 --- a/searchcore/src/vespa/searchcore/proton/server/storeonlyfeedview.cpp +++ b/searchcore/src/vespa/searchcore/proton/server/storeonlyfeedview.cpp @@ -451,11 +451,11 @@ StoreOnlyFeedView::internalUpdate(FeedToken token, const UpdateOperation &updOp) } else { putSummaryNoop(std::move(futureStream), onWriteDone); } - auto task = makeLambdaTask([upd = updOp.getUpdate(), useDocStore, lid, onWriteDone, + auto task = makeLambdaTask([upd = updOp.getUpdate(), useDocStore, lid, is_replay = onWriteDone->is_replay(), promisedDoc = std::move(promisedDoc), promisedStream = std::move(promisedStream), this]() mutable { - makeUpdatedDocument(useDocStore, lid, *upd, std::move(onWriteDone), + makeUpdatedDocument(useDocStore, lid, *upd, is_replay, std::move(promisedDoc), std::move(promisedStream)); }); _writeService.shared().execute(CpuUsage::wrap(std::move(task), CpuUsage::Category::WRITE)); @@ -465,19 +465,19 @@ StoreOnlyFeedView::internalUpdate(FeedToken token, const UpdateOperation &updOp) void StoreOnlyFeedView::makeUpdatedDocument(bool useDocStore, Lid lid, const DocumentUpdate & update, - std::decay_t<OnOperationDoneType> onWriteDone, PromisedDoc promisedDoc, + bool is_replay, PromisedDoc promisedDoc, PromisedStream promisedStream) { Document::UP prevDoc = _summaryAdapter->get(lid, *_repo); Document::UP newDoc; vespalib::nbostream newStream(12345); - assert(onWriteDone->is_replay() || useDocStore); + assert(is_replay || useDocStore); if (useDocStore) { assert(prevDoc); } if (!prevDoc) { // Replaying, document removed later before summary was flushed. - assert(onWriteDone->is_replay()); + assert(is_replay); // If we've passed serial number for flushed index then we could // also check that this operation is marked for ignore by index // proxy. @@ -491,10 +491,9 @@ StoreOnlyFeedView::makeUpdatedDocument(bool useDocStore, Lid lid, const Document } else { // Replaying, document removed and lid reused before summary // was flushed. - assert(onWriteDone->is_replay() && !useDocStore); + assert(is_replay && !useDocStore); } } - onWriteDone.reset(); promisedDoc.set_value(std::move(newDoc)); promisedStream.set_value(std::move(newStream)); } diff --git a/searchcore/src/vespa/searchcore/proton/server/storeonlyfeedview.h b/searchcore/src/vespa/searchcore/proton/server/storeonlyfeedview.h index e36946c214d..2822aa70525 100644 --- a/searchcore/src/vespa/searchcore/proton/server/storeonlyfeedview.h +++ b/searchcore/src/vespa/searchcore/proton/server/storeonlyfeedview.h @@ -184,7 +184,7 @@ private: IPendingLidTracker::Token get_pending_lid_token(const DocumentOperation &op); - void makeUpdatedDocument(bool useDocStore, Lid lid, const DocumentUpdate & update, std::decay_t<OnOperationDoneType> onWriteDone, + void makeUpdatedDocument(bool useDocStore, Lid lid, const DocumentUpdate & update, bool is_replay, PromisedDoc promisedDoc, PromisedStream promisedStream); protected: diff --git a/vespalog/src/test/threads/testthreads.cpp b/vespalog/src/test/threads/testthreads.cpp index f95db5ecc10..af0fe509080 100644 --- a/vespalog/src/test/threads/testthreads.cpp +++ b/vespalog/src/test/threads/testthreads.cpp @@ -6,6 +6,7 @@ #include <iostream> #include <thread> #include <chrono> +#include <atomic> #include <fcntl.h> #include <unistd.h> #include <sys/stat.h> @@ -19,29 +20,29 @@ LOG_SETUP(".threadtest"); class FileThread : public FastOS_Runnable { - bool _done; + std::atomic<bool> _done; string _file; public: FileThread(string file) : _done(false), _file(file) {} void Run(FastOS_ThreadInterface *thread, void *arg) override; - void stop() {_done = true; } + void stop() { _done.store(true, std::memory_order_relaxed); } }; class LoggerThread : public FastOS_Runnable { - bool _done; + std::atomic<bool> _done; public: - bool _useLogBuffer; + std::atomic<bool> _useLogBuffer; LoggerThread() : _done(false), _useLogBuffer(false) {} void Run(FastOS_ThreadInterface *thread, void *arg) override; - void stop() {_done = true; } + void stop() { _done.store(true, std::memory_order_relaxed); } }; void FileThread::Run(FastOS_ThreadInterface *, void *) { unlink(_file.c_str()); - while (!_done) { + while (!_done.load(std::memory_order_relaxed)) { int fd = open(_file.c_str(), O_RDWR | O_CREAT | O_APPEND, 0644); if (fd == -1) { fprintf(stderr, "open failed: %s\n", strerror(errno)); @@ -66,8 +67,8 @@ void LoggerThread::Run(FastOS_ThreadInterface *, void *) { int counter = 0; - while (!_done) { - if (_useLogBuffer) { + while (!_done.load(std::memory_order_relaxed)) { + if (_useLogBuffer.load(std::memory_order_relaxed)) { LOGBM(info, "bla bla bla %u", ++counter); } else { LOG(info, "bla bla bla"); @@ -114,7 +115,7 @@ ThreadTester::Main() } // Then set to use logbuffer and continue for (int i = 0; i < numLoggers; i++) { - loggers[i]->_useLogBuffer = true; + loggers[i]->_useLogBuffer.store(true, std::memory_order_relaxed); } start = steady_clock::now(); while ((steady_clock::now() - start) < 2.5s) { |