read schema from json file #2

larisau · 2018-08-21T15:24:27Z

read schema from json file
support multiple tables in keyspaces
added mode option: write/read/mixed
collecting results enhancement

- support multiple tables in keyspace

larisau · 2018-09-09T07:27:22Z

@penberg can you take a look on this?

penberg · 2018-09-17T05:10:28Z

cmd/gemini/root.go

@@ -19,6 +19,7 @@ var (
 	seed              int
 	dropSchema        bool
 	verbose           bool
+	mode              string


Let's add a type for this.

Go doesn't have built-in enumerations, but I think the idiom looks something like this:

type Mode string const ( ReadMode Mode = "read" MixedMode Mode = "mixed" )

agree, const is better, I'll change it

penberg · 2018-09-17T05:11:34Z

cmd/gemini/root.go

-	return sum
+type Results interface {
+	Merge(*Status) Status
+	Print()
 }


Is this interface used somewhere? If not, let's drop it.

it's used for collecting and printing the results, see the runJob below

penberg · 2018-09-17T05:13:44Z

cmd/gemini/root.go

@@ -79,7 +85,7 @@ func run(cmd *cobra.Command, args []string) {
 		},
 	})
 	schema := schemaBuilder.Build()
-	if dropSchema {
+	if dropSchema && mode != "read" {


Why do we override user's decision to drop schema if mode is "read"?

because now it's read only, so we need data to be read

penberg · 2018-09-17T05:16:45Z

cmd/gemini/root.go

@@ -172,7 +181,8 @@ func init() {
 	rootCmd.MarkFlagRequired("test-cluster")
 	rootCmd.Flags().StringVarP(&oracleClusterHost, "oracle-cluster", "o", "", "Host name of the oracle cluster that provides correct answers")
 	rootCmd.MarkFlagRequired("oracle-cluster")
-	rootCmd.Flags().IntVarP(&maxTests, "max-tests", "m", 100, "Maximum number of test iterations to run")
+	rootCmd.Flags().StringVarP(&mode, "mode", "m", "mixed", "Mode options: write, read, mixed(default)")


Space before parenthesis in the help text.

Perhaps the help text could be improved with something like:

Mode of query operations. Options: write, read, and mixed (default).

penberg · 2018-09-17T05:17:49Z

cmd/gemini/root.go

@@ -22,6 +25,8 @@ var (
 	mode              string
 )

+const confFile = "schema.json"


Why not turn the name of the schema configuration file into a command line option?

we'll do this in the future, when more schema options and data types will be supported

penberg · 2018-09-17T05:19:59Z

cmd/gemini/root.go

+type jsonSchema struct {
+	Keyspace gemini.Keyspace `json:"keyspace"`
+	Tables   []gemini.Table  `json:"tables"`
+}


Why is this separate type for JSON serialization needed? Can't we just make the main gemini.Schema serializable?

Btw, for generality in future patches, we probably ought to move table definitions inside keyspace definitions, and make schema a collection of keyspaces.

yes, we planned it for future

penberg · 2018-09-17T05:24:24Z

cmd/gemini/root.go

+	}
+	defer conf.Close()
+
+	byteValue, err := ioutil.ReadAll(conf)


You can use ioutil.ReadFile to simplify this:

conf, err := ioutil.ReadFile(confFile) var schema jsonSchema err := json.Unmarshal(conf, &schema)

thanks, really - ReadFile has open and close inside

penberg · 2018-09-17T05:29:31Z

schema.go

+	case "text", "varchar":
+		values = append(values, randString(randRange(p.Min, p.Max)))
+	case "timestamp", "date":
+		values = append(values, randDate())


We could also add (in future patches) varint (arbitrary precision integers) using https://golang.org/pkg/math/big/

penberg · 2018-09-17T05:32:56Z

schema.go

+	day := randRange(1, 30)
+	month := randRange(1, 12)
+	year := randRange(2000, 2018)
+	return time.Date(year, time.Month(month), day, rand.Intn(24), rand.Intn(60), rand.Intn(60), 0, time.UTC)


This generates incorrect dates such as "2018-02-30" (there are not that many days in February) and doesn't take account leap years and so on.

It is easier to generate a random number representing seconds since epoch and use time.Unix to turn that into a time.Time type.

If needed (is it really?), we can limit the minimum and maximum dates as follows, for example:

https://stackoverflow.com/a/43497333

penberg · 2018-09-17T05:35:58Z

schema.go

 	case "int_range":
 		start := randRange(p.Min, p.Max)
 		end := start + randRange(p.Min, p.Max)
 		values = append(values, start)
 		values = append(values, end)
-	case "blob":
+	case "blob", "uuid":
 		r, _ := uuid.NewRandom()
 		values = append(values, r.String())


I guess we need to improve blob value generation in a follow up to be something else then that better represents what blobs are used for. Making blogs significantly larger, for example, will make things less easy for Scylla and perhaps uncover some bugs.

yes, it's in our plans

larisau · 2018-09-23T09:38:06Z

@penberg can we merge it for now?

avikivity · 2018-09-23T10:40:35Z

I don't understand read-only or write-only modes. Read-only won't read anything because there's nothing there, and write-only won't verify anything.

larisau · 2018-09-23T12:30:03Z

@avikivity The write mode can be used to populate the same data in both clusters, the read mode compares data between the two using randomly generated queries, the the mixed mode does exactly the same - it just always runs read after write.

avikivity · 2018-09-23T12:33:34Z

reads and writes should be run in parallel. read-after-write is too simple.

larisau · 2018-09-24T19:23:25Z

read-after-write jobs are running in parallel - each one works with its partition range.
In our plans:

save on the "oracle" pattern only
writes protected by lock, so no need for partition range

avikivity · 2018-09-25T09:40:26Z

Reads should be intermingled with writes, to the same partition and clustering keys. That's what real applications do.

larisau requested a review from penberg August 21, 2018 15:24

larisau added 2 commits August 26, 2018 16:14

added mode option: write/read/mixed

f708b6c

- read schema from json file

6444133

- support multiple tables in keyspace

larisau force-pushed the mode_options branch from 3385f2d to 6444133 Compare August 26, 2018 13:16

larisau requested review from noamha and mmatczuk August 26, 2018 13:17

larisau changed the title ~~added mode option: write/read/mixed~~ read schema from json file Aug 26, 2018

larisau force-pushed the mode_options branch from 51d431b to 1a07722 Compare August 28, 2018 13:02

penberg reviewed Sep 17, 2018

View reviewed changes

support more data types

5daad67

larisau force-pushed the mode_options branch from 1a07722 to 5daad67 Compare September 23, 2018 09:23

larisau merged commit 7309af4 into master Oct 2, 2018

larisau deleted the mode_options branch October 22, 2018 09:41

CodeLieutenant mentioned this pull request Nov 30, 2024

Gemini CQL statement logging generates large files #441

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

read schema from json file #2

read schema from json file #2

larisau commented Aug 21, 2018 •

edited

Loading

larisau commented Sep 9, 2018

penberg Sep 17, 2018

larisau Sep 23, 2018

larisau Sep 23, 2018

penberg Sep 17, 2018

larisau Sep 23, 2018

penberg Sep 17, 2018

larisau Sep 23, 2018

penberg Sep 17, 2018

larisau Sep 23, 2018

penberg Sep 17, 2018

larisau Sep 23, 2018

penberg Sep 17, 2018 •

edited

Loading

larisau Sep 23, 2018

penberg Sep 17, 2018

larisau Sep 23, 2018

larisau Sep 23, 2018

penberg Sep 17, 2018

larisau Sep 23, 2018

penberg Sep 17, 2018

larisau Sep 23, 2018

penberg Sep 17, 2018

larisau Sep 23, 2018

larisau commented Sep 23, 2018

avikivity commented Sep 23, 2018

larisau commented Sep 23, 2018

avikivity commented Sep 23, 2018

larisau commented Sep 24, 2018 •

edited

Loading

avikivity commented Sep 25, 2018

read schema from json file #2

read schema from json file #2

Conversation

larisau commented Aug 21, 2018 • edited Loading

larisau commented Sep 9, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

penberg Sep 17, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

larisau commented Sep 23, 2018

avikivity commented Sep 23, 2018

larisau commented Sep 23, 2018

avikivity commented Sep 23, 2018

larisau commented Sep 24, 2018 • edited Loading

avikivity commented Sep 25, 2018

larisau commented Aug 21, 2018 •

edited

Loading

penberg Sep 17, 2018 •

edited

Loading

larisau commented Sep 24, 2018 •

edited

Loading