scylladb / cql-stress

10 stars 4 forks source link

c-s: create correct tables based on workload #86

Closed muzarski closed 3 months ago

muzarski commented 3 months ago

fix: https://github.com/scylladb/cql-stress/issues/85

List of tables created for each command:

muzarski commented 3 months ago

v2: addressed review comment

roydahan commented 3 months ago
  • mixed -> both keyspace1.[standard1/counter1] tables

@muzarski the defined behavior is incorrect. Actually mixed should be the same as "read/write" and run on table keyspace1.standard1. The difference is that mixed should do 50/50 reads/writes ops.

muzarski commented 3 months ago
  • mixed -> both keyspace1.[standard1/counter1] tables

@muzarski the defined behavior is incorrect. Actually mixed should be the same as "read/write" and run on table keyspace1.standard1. The difference is that mixed should do 50/50 reads/writes ops.

cassandra-stress help mixed says:

ratio([read=?][write=?][counter_write=?][counter_read=?]): Specify the ratios for operations to perform; e.g. (read=2,write=1) will perform 2 reads for each write
      read=? (default=1)                       Performs this many READ operations out of total
      write=? (default=1)                      Performs this many WRITE operations out of total
      counter_write=?                          Performs this many COUNTER_WRITE operations out of total
      counter_read=?                           Performs this many COUNTER_READ operations out of total

This means that counter_write and counter_read operations can be performed as well. Thus, the table keyspace1.counter1 is being used as well.

muzarski commented 3 months ago

The difference is that mixed should do 50/50 reads/writes ops.

The 50/50 reads/writes is the default behaviour, but can be configured via ratio parameter (so the tool performs operations on counter tables as well).

muzarski commented 3 months ago

ping @roydahan - I'd like to make sure we are on the same page so I can merge this.

roydahan commented 3 months ago

Ok, I understand now. So, as long as the default ratio is only "read,write" it should be fine.

roydahan commented 3 months ago

@soyacz I asked @muzarski to merge and release so we can retest the tablets one without even creating the table. The "mixed" could support counters workload but you must explictily state a ratio that include counter_write, counter_read.

roydahan commented 3 months ago

@muzarski rethinking of it, AFAIR in c-s we don't create the tables in "mixed" command, it assumes that one already ran a "write" command before that in order to prepare for it. So, unless it's an improvement we're adding on top of c-s, cql-stress shouldn't do it as well.

muzarski commented 3 months ago

@muzarski rethinking of it, AFAIR in c-s we don't create the tables in "mixed" command, it assumes that one already ran a "write" command before that in order to prepare for it. So, unless it's an improvement we're adding on top of c-s, cql-stress shouldn't do it as well.

Oh, that's interesting. But it would make sense, since mixed allows to execute read operation (which requires a populated DB). Let me verify this, and I'll adjust the logic properly for cql-stress.

muzarski commented 3 months ago

v3: List of tables created for each command:

@roydahan Please confirm that it makes sense now.

fruch commented 3 months ago

@muzarski

now we don't create the schema...

but the code still tries to prepare statements on the non-existing keyspace/table..., which is even worse, mixed can't be use now...

TAG: loader_idx:4-cpu_idx:0-keyspace_idx:1
******************** Stress Settings ********************
Command:
  Type: mixed
  Count: -1
  Duration: 48000 SECONDS
  No Warmup: true
  Consistency Level: Quorum
  Serial Consistency Level: Serial
  Truncate: NEVER
  Target Uncertainty: not applicable
  Key Size (bytes): 10
  Counter Increment Distibution: FIXED(1)
Command ratios: {read=1,write=1}
Command clustering distribution: GAUSSIAN(1..10,mean=5.5,stdev=1.5)
Rate:
  Thread count: 300
  OpsPer Sec: 8750
  Coordinated-Omission-Fixed latencies: true
Mode:
  Compression: None
  Pool size: PerShard(1)
Node:
  Nodes: ["10.4.1.151", "10.4.3.241", "10.4.3.226"]
  Is White List: false
  Datacenter: None
Schema:
  Keyspace: keyspace1
  Replication Strategy Options: {"replication_factor": "3", "class": "NetworkTopologyStrategy"}
  Table Compression: None
  Table Compaction Options: {}
Column:
  Column names: ["C0", "C1", "C2", "C3", "C4", "C5", "C6", "C7"]
  Size distribution: FIXED(128)
Population:
  Partition key seed distribution: GAUSSIAN(1..650000000,mean=325000000,stdev=6500000)

Error: Failed to prepare benchmark

Caused by:
    0: Failed to prepare statement
    1: Database returned an error: The query is syntactically correct but invalid, Error message: unconfigured table counter1
muzarski commented 3 months ago

@muzarski rethinking of it, AFAIR in c-s we don't create the tables in "mixed" command, it assumes that one already ran a "write" command before that in order to prepare for it.

So, unless it's an improvement we're adding on top of c-s, cql-stress shouldn't do it as well.

@fruch WDYT?

muzarski commented 3 months ago

I can reverse the changes regarding mixed workload so it creates the schema.

muzarski commented 3 months ago

Looking at the code of original c-s:

    public void maybeCreateKeyspaces()
    {
        if (command.type == Command.WRITE || command.type == Command.COUNTER_WRITE)
            schema.createKeySpaces(this);
        else if (command.type == Command.USER)
            ((SettingsCommandUser) command).profile.maybeCreateSchema(this);
    }

Keyspace and tables are not created for mixed workloads. @fruch Please try to run the write workload firstly to create a schema. Or if this really annoying and you think that we should create schema during mixed command as well, please open an issue about it so we can discuss it there.

fruch commented 3 months ago

Looking at the code of original c-s:

    public void maybeCreateKeyspaces()
    {
        if (command.type == Command.WRITE || command.type == Command.COUNTER_WRITE)
            schema.createKeySpaces(this);
        else if (command.type == Command.USER)
            ((SettingsCommandUser) command).profile.maybeCreateSchema(this);
    }

Keyspace and tables are not created for mixed workloads. @fruch Please try to run the write workload firstly to create a schema. Or if this really annoying and you think that we should create schema during mixed command as well, please open an issue about it so we can discuss it there.

We did run the write workload before, but the mixed workload shouldn't do any operations with counters, if we want counter there is a command for that.

Now tablets doesn't support counters, and we need that that read/write/mixed won't even create the counter schema, and not do any counter operations

muzarski commented 3 months ago

Looking at the code of original c-s:

    public void maybeCreateKeyspaces()
    {
        if (command.type == Command.WRITE || command.type == Command.COUNTER_WRITE)
            schema.createKeySpaces(this);
        else if (command.type == Command.USER)
            ((SettingsCommandUser) command).profile.maybeCreateSchema(this);
    }

Keyspace and tables are not created for mixed workloads. @fruch Please try to run the write workload firstly to create a schema. Or if this really annoying and you think that we should create schema during mixed command as well, please open an issue about it so we can discuss it there.

We did run the write workload before, but the mixed workload shouldn't do any operations with counters, if we want counter there is a command for that.

Now tablets doesn't support counters, and we need that that read/write/mixed won't even create the counter schema, and not do any counter operations

Oh, I see. We try to prepare a statement operating on counter1 table. This is indeed a bug. I'll try to fix that ASAP.