Skip to content

Cardinality and consistency issues among test participants #102

Description

@gandaldf

After carefully reviewing the results of the latest (local) test, I noticed that some participants had (or continued to have) unusually "positive results".
Looking more closely at how their tests were implemented, I found that there are cardinality issues and that the test suite places a great deal of trust in the participants, without actually verifying what they write, read, or modify.
In practice, we never check "RowsAffected" or the IDs returned or the length of ReadSlice, or whether the values are actually materialized. We simply consider the operation successful as long as no error is returned.

In the specific case I analyzed, reform is effectively benchmarking Update instead of Insert, because Save(m) inserts a new element only when the ID is zero. As a result, after the first iteration, all subsequent iterations are just updates to the same element. This also means that only 1 element is created instead of 100, significantly skewing the benchmark results.

I would suggest introducing the smallest possible change (even though it would eventually need to be applied to all participants).
For example, we could add two helper functions, BenchExec and BenchRows, whose purpose would be to centralize ReportAllocs, ResetTimer, StartTimer, the b.N loop, StopTomer, and the recording of the first error, stopping the benchmark when necessary.
With these two helpers, we could fully manage the timer and the b.N loop, reset and validate the database state, and verify the number of rows materialized by Read and ReadSlice.
Ideally, I would expect the test functions to end up looking something like this:

func (raw *Raw) Insert(b *testing.B) {
    m := NewModel()

    helper.BenchExec(b, raw.Name(), "Insert", func() error {
        _, err := raw.conn.Exec(
            rawInsertSQL,
            m.Name,
            m.Title,
            m.Fax,
            m.Web,
            m.Age,
            m.Right,
            m.Counter,
        )
        return err
    })
}

and for a hypothetical ReadSlice, something like this:

helper.BenchRows(b, raw.Name(), "ReadSlice", helper.BatchSize, func() (int, error) {
        rows, err := raw.conn.Query(query)
        if err != nil {
            return 0, err
        }
        defer rows.Close()

        scanned := 0
        for rows.Next() {
            // Scan
            scanned++
        }

        return scanned, rows.Err()
    },
)

The ORMInterface can remain unchanged: the checks are performed inside the helpers and the suite wrapper.

What do you think, @efectn?

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions