Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

workload/ycsb: implement read-modify-write #39430

Merged
merged 1 commit into from
Aug 8, 2019

Conversation

jeffrey-xiao
Copy link
Contributor

Release note: None

@cockroach-teamcity
Copy link
Member

This change is Reviewable

@jeffrey-xiao jeffrey-xiao force-pushed the ycsb-read-modify-insert branch from d8e3b39 to fd27d52 Compare August 8, 2019 01:46
Copy link
Contributor

@ajwerner ajwerner left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice

Reviewed 1 of 1 files at r1.
Reviewable status: :shipit: complete! 0 of 0 LGTMs obtained (waiting on @jeffrey-xiao and @nvanbenschoten)


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

			return err
		}
		for res.Next() {

Should this ever read more than one row? If not you could do QueryRowContext

Copy link
Member

@nvanbenschoten nvanbenschoten left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

:lgtm:

Reviewable status: :shipit: complete! 1 of 0 LGTMs obtained (waiting on @jeffrey-xiao)


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

	}

	readStmts := make([]*gosql.Stmt, numTableFields)

nit: move up next to readStmt and rename to readFieldStmts. Same thing in the ycsbWorker struct.

Copy link
Member

@nvanbenschoten nvanbenschoten left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewable status: :shipit: complete! 1 of 0 LGTMs obtained (waiting on @jeffrey-xiao)


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

	value := yw.randString(fieldLength)
	fieldIdx := yw.rng.Intn(numTableFields)
	args := make([]interface{}, 2)

You can use an array here without it escaping. Could you do that here, in insertRow, and in updateRow? Something like:

diff --git a/pkg/workload/ycsb/ycsb.go b/pkg/workload/ycsb/ycsb.go
index 4d92b018db..f2d9356556 100644
--- a/pkg/workload/ycsb/ycsb.go
+++ b/pkg/workload/ycsb/ycsb.go
@@ -484,12 +484,12 @@ func (yw *ycsbWorker) randString(length int) string {
 }

 func (yw *ycsbWorker) insertRow(ctx context.Context, keyIndex uint64, increment bool) error {
-       args := make([]interface{}, numTableFields+1)
+       var args [numTableFields + 1]interface{}
        args[0] = yw.buildKeyName(keyIndex)
        for i := 1; i <= numTableFields; i++ {
                args[i] = yw.randString(fieldLength)
        }
-       if _, err := yw.insertStmt.ExecContext(ctx, args...); err != nil {
+       if _, err := yw.insertStmt.ExecContext(ctx, args[:]...); err != nil {
                yw.nextInsertIndex = new(uint64)
                *yw.nextInsertIndex = keyIndex
                return err
@@ -509,7 +509,7 @@ func (yw *ycsbWorker) insertRow(ctx context.Context, keyIndex uint64, increment

 func (yw *ycsbWorker) updateRow(ctx context.Context) error {
        var stmt *gosql.Stmt
-       args := make([]interface{}, 2)
+       var args [2]interface{}
        args[0] = yw.nextReadKey()
        fieldIdx := yw.rng.Intn(numTableFields)
        value := yw.randString(fieldLength)
@@ -520,7 +520,7 @@ func (yw *ycsbWorker) updateRow(ctx context.Context) error {
                stmt = yw.updateStmts[fieldIdx]
                args[1] = value
        }
-       if _, err := stmt.ExecContext(ctx, args...); err != nil {
+       if _, err := stmt.ExecContext(ctx, args[:]...); err != nil {
                return err
        }
        return nil

@jeffrey-xiao jeffrey-xiao force-pushed the ycsb-read-modify-insert branch 2 times, most recently from 5838477 to d2d038f Compare August 8, 2019 19:58
Copy link
Member

@nvanbenschoten nvanbenschoten left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewable status: :shipit: complete! 0 of 0 LGTMs obtained (and 1 stale) (waiting on @ajwerner and @jeffrey-xiao)


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

func (yw *ycsbWorker) insertRow(ctx context.Context) error {
	var args [numTableFields]interface{}

numTableFields + 1

@jeffrey-xiao jeffrey-xiao force-pushed the ycsb-read-modify-insert branch from d2d038f to 7e32672 Compare August 8, 2019 20:08
@jeffrey-xiao jeffrey-xiao force-pushed the ycsb-read-modify-insert branch from 7e32672 to 6bec279 Compare August 8, 2019 20:59
@jeffrey-xiao
Copy link
Contributor Author

TFTRs!

bors r+

craig bot pushed a commit that referenced this pull request Aug 8, 2019
39375: exec: add null handling to default sorter r=rohany a=rohany

Addresses part of #36880.

Release note: None

39420: roachtest: fix pgjdbc tests r=rafiss a=rafiss

The pgjdbc tests have two types of failures: one referred to as
"failures" and the other one as "errors." We were only counting the
former before. Now there are many more test failures that should be
tracked in the blacklist.

touches #39406 

Release note: None

39430: workload/ycsb: implement read-modify-write r=jeffrey-xiao a=jeffrey-xiao

Release note: None

39461: internal/sqlsmith: add vectorize option r=mjibson a=mjibson

This option attempts to limits query generation such that vectorized
execution will occur. Still a lot of work to do but this is a good start.

Release note: None

Co-authored-by: Rohan Yadav <[email protected]>
Co-authored-by: Rafi Shamim <[email protected]>
Co-authored-by: Jeffrey Xiao <[email protected]>
Co-authored-by: Matt Jibson <[email protected]>
@craig
Copy link
Contributor

craig bot commented Aug 8, 2019

Build succeeded

@craig craig bot merged commit 6bec279 into cockroachdb:master Aug 8, 2019
@jeffrey-xiao jeffrey-xiao deleted the ycsb-read-modify-insert branch August 15, 2019 17:47
ajwerner added a commit to ajwerner/cockroach that referenced this pull request Feb 7, 2020
In cockroachdb#39430 we enhanced our workload ycsb implementation. In the process we broke
the json functionality. This fixes it. If you want tests I'll write them later.

Release note: None
craig bot pushed a commit that referenced this pull request Feb 8, 2020
44879: workload: fix ycsb -json r=nvanbenschoten a=ajwerner

In #39430 we enhanced our workload ycsb implementation. In the process we broke
the json functionality. This fixes it. If you want tests I'll write them later.

Release note: None

Co-authored-by: Andrew Werner <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants