Skip to content

Conversation

@andregmoeller
Copy link
Contributor

This is not a pull request, which improves the performance. It is meant to clean up the go-raw-mongodb implementation.
I did the following changes:

  • Fix warning concerning the content-type in fortuneHandler
  • Add additional error handling to fortuneHandler
  • make sure that the identity column is _id (see http://frameworkbenchmarks.readthedocs.org/en/latest/Project-Information/Framework-Tests/: ...except for MongoDB, wherein the identity column is _id, with the leading underscore)
  • removed runtime.GOMAXPROCS(runtime.NumCPU()) (see https://golang.org/doc/go1.5: By default, Go programs run with GOMAXPROCS set to the number of cores available; in prior releases it defaulted to 1.)
  • removed unnecessary if/else/return (log.Fatal/log.Fatalf is equivalent to Print()/Printf() followed by a call to os.Exit(1).)
  • simplified the dbHandler and queriesHandler
  • In my opinion the original implementation of the Database update test violated requirement numbers 4, 5 and 6 - in my opinion the original code didn't read the original randomNumber field from the database:
    • The request handler must retrieve a set of World objects, equal in count to the queries parameter, from the World database table.
    • Each row must be selected randomly using one query in the same fashion as the single database query test (Test Benchmarking tool suggestion. #2 above). As with the read-only multiple-query test type (Update and improve Compojure #3 above), use of IN clauses or similar means to consolidate multiple queries into one operation is not permitted.
    • At least the randomNumber field must be read from the database result set.

@ssmith-techempower
Copy link
Contributor

Looks good to me, merging in.

ssmith-techempower added a commit that referenced this pull request Mar 8, 2016
@ssmith-techempower ssmith-techempower merged commit 8559f8f into TechEmpower:master Mar 8, 2016
@andregmoeller andregmoeller deleted the go-raw-mongodb-clean-up branch March 9, 2016 08:59
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.

2 participants