workload/ycsb: add support for uniform load distribution

[m-schneider]

Previously we could only run YCSB with a zipfian distribution, now we'll
be able to use a default uniform distribution.

Closes #30996

Release note: None

[cockroach-teamcity]

This change is 

[cockroach-teamcity]

This change is 

[nvanbenschoten]

Reviewed 1 of 2 files at r1.
Reviewable status: complete! 0 of 0 LGTMs obtained

---

pkg/workload/ycsb/uniform_generator.go, line 20 at r1 (raw file):

	mu       syncutil.Mutex
	r        *rand.Rand
	sequence uint64

I'm a little confused about this sequence number. Should we be setting it initially? Are we going to repeatedly call IncrementIMax() until its where we want it? Does it need to be dynamic or can we just set it once?

---

pkg/workload/ycsb/uniform_generator.go, line 48 at r1 (raw file):

}

// IncrementIMax increments, iMax, and recompute the internal values that depend

Don't need the commas in here.

---

pkg/workload/ycsb/ycsb.go, line 242 at r1 (raw file):

	if strings.ToLower(g.distribution) == "zipfian" {
		randGen, err = NewZipfGenerator(
			zipfRng, zipfIMin, defaultIMax, defaultTheta, false) /* verbose */

Looks like the inline comment got messed up.

---

pkg/workload/ycsb/ycsb.go, line 245 at r1 (raw file):

	} else {
		randGen, err = NewUniformGenerator(zipfRng, zipfIMin)
	}

Let's throw an error if a different distribution is given.

---

pkg/workload/ycsb/ycsb.go, line 268 at r1 (raw file):

}

type RandGenerator interface {

No need to export this.

[m-schneider]

Reviewable status: complete! 0 of 0 LGTMs obtained

---

pkg/workload/ycsb/uniform_generator.go, line 20 at r1 (raw file):

Previously, nvanbenschoten (Nathan VanBenschoten) wrote…

I'm a little confused about this sequence number. Should we be setting it initially? Are we going to repeatedly call IncrementIMax() until its where we want it? Does it need to be dynamic or can we just set it once?

So this is technically more for when we get proper inserts working with both distributions. I'm using sequence as a row count. I didn't want to touch the zipfian stuff as much as possible so I kept the function names.

---

pkg/workload/ycsb/uniform_generator.go, line 48 at r1 (raw file):

Previously, nvanbenschoten (Nathan VanBenschoten) wrote…

Don't need the commas in here.

Changed the comment

---

pkg/workload/ycsb/ycsb.go, line 242 at r1 (raw file):

Previously, nvanbenschoten (Nathan VanBenschoten) wrote…

Looks like the inline comment got messed up.

Hmm you're right the ) is on the wrong side.

---

pkg/workload/ycsb/ycsb.go, line 245 at r1 (raw file):

Previously, nvanbenschoten (Nathan VanBenschoten) wrote…

Let's throw an error if a different distribution is given.

Done.

---

pkg/workload/ycsb/ycsb.go, line 268 at r1 (raw file):

Previously, nvanbenschoten (Nathan VanBenschoten) wrote…

No need to export this.

Done.

[nvanbenschoten]

Reviewable status: complete! 0 of 0 LGTMs obtained

---

pkg/workload/ycsb/uniform_generator.go, line 20 at r1 (raw file):

Previously, m-schneider (Masha Schneider) wrote…

So this is technically more for when we get proper inserts working with both distributions. I'm using sequence as a row count. I didn't want to touch the zipfian stuff as much as possible so I kept the function names.

But unlike the zipfian distribution, we don't give it an initial value, so how does this work when we're not inserting?

[m-schneider]

---

pkg/workload/ycsb/uniform_generator.go, line 20 at r1 (raw file):

Previously, nvanbenschoten (Nathan VanBenschoten) wrote…

But unlike the zipfian distribution, we don't give it an initial value, so how does this work when we're not inserting?

I'm passing it the initial number or rows. To support inserts the zipfian will need a similar mechanism.

[m-schneider]

---

pkg/workload/ycsb/uniform_generator.go, line 20 at r1 (raw file):

Previously, nvanbenschoten (Nathan VanBenschoten) wrote…

But unlike the zipfian distribution, we don't give it an initial value, so how does this work when we're not inserting?

I'm passing it the initial number or rows. To support inserts the zipfian will need a similar mechanism.

[asubiotto]

Reviewable status: complete! 0 of 0 LGTMs obtained

---

pkg/workload/ycsb/uniform_generator.go, line 12 at r2 (raw file):

type UniformGenerator struct {
	// The underlying RNG
	uniformGenMu UniformGeneratorMU

Could you use an anonymous struct here? i.e.

type UniformGenerator struct {
    minValue uint64
    mu struct {
        syncutil.Mutex
        r *rand.Rand
        ...
    }
}

[m-schneider]

---

pkg/workload/ycsb/uniform_generator.go, line 12 at r2 (raw file):

Previously, asubiotto (Alfonso Subiotto Marqués) wrote…

Could you use an anonymous struct here? i.e.

type UniformGenerator struct {
    minValue uint64
    mu struct {
        syncutil.Mutex
        r *rand.Rand
        ...
    }
}

Much cleaner.

[m-schneider]

---

pkg/workload/ycsb/uniform_generator.go, line 12 at r2 (raw file):

Previously, asubiotto (Alfonso Subiotto Marqués) wrote…

Could you use an anonymous struct here? i.e.

type UniformGenerator struct {
    minValue uint64
    mu struct {
        syncutil.Mutex
        r *rand.Rand
        ...
    }
}

Much cleaner.

[nvanbenschoten]

Reviewable status: complete! 0 of 0 LGTMs obtained

---

pkg/workload/ycsb/uniform_generator.go, line 20 at r1 (raw file):

Previously, m-schneider (Masha Schneider) wrote…

I'm passing it the initial number or rows. To support inserts the zipfian will need a similar mechanism.

As the minimum value? Doesn't that mean that reads will never read existing data?

[m-schneider]

Reviewable status: complete! 0 of 0 LGTMs obtained

---

pkg/workload/ycsb/uniform_generator.go, line 20 at r1 (raw file):

Previously, nvanbenschoten (Nathan VanBenschoten) wrote…

As the minimum value? Doesn't that mean that reads will never read existing data?

You're absolutely right! The min value is more as a lower bound for the random number generated for reads, however because the current zipfian generator isn't quite correct, it mods the result by the number of rows.

[m-schneider]

---

pkg/workload/ycsb/ycsb.go, line 355 at r3 (raw file):

	// this should work. (Basically repeatedly drawing from the distribution until
	// a sufficiently low value is chosen, but with some complications.)
	rownum := yw.hashKey(yw.randGen.Uint64()) % uint64(yw.config.initialRows)

Tested the uniform distribution via roachtest, the only strange thing is this hashKey method. It's not in the original YCSB code. Do you know what this is?

[m-schneider]

---

pkg/workload/ycsb/ycsb.go, line 355 at r3 (raw file):

	// this should work. (Basically repeatedly drawing from the distribution until
	// a sufficiently low value is chosen, but with some complications.)
	rownum := yw.hashKey(yw.randGen.Uint64()) % uint64(yw.config.initialRows)

Tested the uniform distribution via roachtest, the only strange thing is this hashKey method. It's not in the original YCSB code. Do you know what this is?

[nvanbenschoten]

Reviewed 1 of 2 files at r3.
Reviewable status: complete! 0 of 0 LGTMs obtained

---

pkg/workload/ycsb/ycsb.go, line 355 at r3 (raw file):

Previously, m-schneider (Masha Schneider) wrote…

Tested the uniform distribution via roachtest, the only strange thing is this hashKey method. It's not in the original YCSB code. Do you know what this is?

No, I'm not sure about it. It's strange because it looks like we actually hash the random number twice - once here and once in buildKeyName. Can we just remove this entirely?

[m-schneider]

---

pkg/workload/ycsb/ycsb.go, line 355 at r3 (raw file):

Previously, nvanbenschoten (Nathan VanBenschoten) wrote…

No, I'm not sure about it. It's strange because it looks like we actually hash the random number twice - once here and once in buildKeyName. Can we just remove this entirely?

I'd be nervous moving it out because we could break the Zipfian distribution accidentally. Maybe I could pass the function to the distributions and have uniform ignore it and zipfian use it? When I ran uniform on 5 rows with it, it gave 0 and 4 as the only two choices.

[m-schneider]

---

pkg/workload/ycsb/ycsb.go, line 355 at r3 (raw file):

Previously, nvanbenschoten (Nathan VanBenschoten) wrote…

No, I'm not sure about it. It's strange because it looks like we actually hash the random number twice - once here and once in buildKeyName. Can we just remove this entirely?

I'd be nervous moving it out because we could break the Zipfian distribution accidentally. Maybe I could pass the function to the distributions and have uniform ignore it and zipfian use it? When I ran uniform on 5 rows with it, it gave 0 and 4 as the only two choices.

[nvanbenschoten]

Reviewable status: complete! 0 of 0 LGTMs obtained

---

pkg/workload/ycsb/ycsb.go, line 355 at r3 (raw file):

Previously, m-schneider (Masha Schneider) wrote…

I'd be nervous moving it out because we could break the Zipfian distribution accidentally. Maybe I could pass the function to the distributions and have uniform ignore it and zipfian use it? When I ran uniform on 5 rows with it, it gave 0 and 4 as the only two choices.

Hashing twice is nonsensical though. I guess we can leave this for now and ask @arjunravinarayan when he's back. I'd just leave a TODO for now.

[m-schneider]

---

pkg/workload/ycsb/ycsb.go, line 355 at r3 (raw file):

Previously, nvanbenschoten (Nathan VanBenschoten) wrote…

Hashing twice is nonsensical though. I guess we can leave this for now and ask @arjunravinarayan when he's back. I'd just leave a TODO for now.

Yep agreed, added the TODO.

[m-schneider]

---

pkg/workload/ycsb/ycsb.go, line 355 at r3 (raw file):

Previously, nvanbenschoten (Nathan VanBenschoten) wrote…

Hashing twice is nonsensical though. I guess we can leave this for now and ask @arjunravinarayan when he's back. I'd just leave a TODO for now.

Yep agreed, added the TODO.

[m-schneider]

bors r+

[m-schneider]

bors r+

[craig]

Build succeeded

• GitHub CI (Cockroach)

[craig]

Build succeeded

• GitHub CI (Cockroach)