TPC-Council / HammerDB

HammerDB Database Load Testing and Benchmarking Tool
http://www.hammerdb.com
GNU General Public License v3.0
589 stars 119 forks source link

Add Support for oceanbase tpch #670

Closed yaqi-zhao closed 7 months ago

yaqi-zhao commented 7 months ago

As described in https://github.com/TPC-Council/HammerDB/issues/563. This PR support Oceanbase TPROC-H by adding a compatible parameter.

image

Co-authored-by: XiTang xi.tang@intel.com

TPROC-H

Check 1 Database Exist

Check 2 Tables Exist

Check 3 supplier count in schema is the same as dict setting

Check 4 Tables are indexed

Check 5 Tables are populated

Consistency check

sm-shaw commented 7 months ago

Many thanks for the pull request.

I will test to make sure everything is OK with the existing MySQL before approving.

From a quick overview there is a few minor changes to make and I will look to add those to the PR, such as keeping the scale factor and virtual users to build schema at the bottom of the options and also keeping all of the mysql dict options starting with mysql_.

We are currently just about to release v4.10 which has already been tagged and drafted and therefore once merged this is likely to be included with v4.11.

xtangxtang commented 7 months ago

I've tested and added an additional commit to make some changes, what I have changed is:

  1. Updated some of the variable names for consistency with mysql prefix.
  2. It looks the partitions option is only needed in the build script and not the driver i.e. the variable wasn't used in the driver. So I've removed the partitions option from the driver and left it in the build.
  3. I've moved the fields around for consistency across HammerDB.
  4. The changes to ConnectToMySQL always set the user to oceanbase if the local socket was not used when logging in. I've fixed this so if not using oceanbase and are using a port to connect it still works.
  5. The transaction counter was changed to work against oceanbase, however the GUI dialog did then not allow an oceanbase option for the transaction counter - I've fixed this by adding options to the transaction counter.
  6. Cloud Analytic queries errored as they didn't include Oceanbase variables in the EDITABLE OPTIONS - this has been fixed.

I have tested against regular MySQL only to make sure all the TPC-H options work when you don't select the oceanbase options, so am happy to approve on this basis.

Please check to make sure that everything continues to work on the oceanbase side after the updates.

Thanks Steve, we will check if the oceanbase is corret. Any process we will update here.

yaqi-zhao commented 7 months ago

I've tested and added an additional commit to make some changes, what I have changed is:

  1. Updated some of the variable names for consistency with mysql prefix.
  2. It looks the partitions option is only needed in the build script and not the driver i.e. the variable wasn't used in the driver. So I've removed the partitions option from the driver and left it in the build.
  3. I've moved the fields around for consistency across HammerDB.
  4. The changes to ConnectToMySQL always set the user to oceanbase if the local socket was not used when logging in. I've fixed this so if not using oceanbase and are using a port to connect it still works.
  5. The transaction counter was changed to work against oceanbase, however the GUI dialog did then not allow an oceanbase option for the transaction counter - I've fixed this by adding options to the transaction counter.
  6. Cloud Analytic queries errored as they didn't include Oceanbase variables in the EDITABLE OPTIONS - this has been fixed.

I have tested against regular MySQL only to make sure all the TPC-H options work when you don't select the oceanbase options, so am happy to approve on this basis.

Please check to make sure that everything continues to work on the oceanbase side after the updates.

Thanks Steve, we have tested the oceanbase with the newest updates, everything continues to work fine.

sm-shaw commented 7 months ago

Excellent, once fully approved by all reviewers we will merge the pull request