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

GORA-532: Apache Gora Benchmark Module #179

Merged
merged 36 commits into from Sep 8, 2019
Merged

Conversation

@sneceesay77
Copy link
Contributor

sneceesay77 commented Aug 4, 2019

Initial pull request.

Copy link
Contributor

lewismc left a comment

Hi @sneceesay77 this is an excellent conteribution. I've not attempted to run it yet as i am not really sure how to. Can you consider adding a README to the module which talks through how one would run it?
Please see my other comments. They concern pretty trivial things mostly regarding convention.
Also, your logic to auto-generate the XML mapping files is excellent. I think that this could be added to the gora-compiler module instread. I believe there is actually a JIRA tricket open for this.

So far, excellent work :)

gora-benchmark/gora-bench.sh Outdated Show resolved Hide resolved
gora-benchmark/gora-bench.sh Outdated Show resolved Hide resolved
gora-benchmark/pom.xml Show resolved Hide resolved
gora-benchmark/pom.xml Outdated Show resolved Hide resolved
gora-benchmark/pom.xml Outdated Show resolved Hide resolved
@lewismc

This comment has been minimized.

Copy link
Contributor

lewismc commented Aug 7, 2019

Please ping me here @sneceesay77 when the PR updated and you've provided a README of how we can run this. Thank you

@sneceesay77

This comment has been minimized.

Copy link
Contributor Author

sneceesay77 commented Aug 7, 2019

@lewismc , please see my latest update to the pull request. I have added a README file detailing how to run the benchmark module.

@sneceesay77 sneceesay77 closed this Aug 7, 2019
@sneceesay77 sneceesay77 reopened this Aug 7, 2019
@sneceesay77

This comment has been minimized.

Copy link
Contributor Author

sneceesay77 commented Aug 9, 2019

Please ping me here @sneceesay77 when the PR updated and you've provided a README of how we can run this. Thank you

@lewismc, were you able to look at this for me? I have completed all the requested changes.

Copy link
Contributor

lewismc left a comment

@sneceesay77 thanks for updating, this is a huge improvement.
Please comment on the unresolved dependency issue as i think this needs to be resolved before we move on.

gora-benchmark/README.md Outdated Show resolved Hide resolved
gora-benchmark/README.md Outdated Show resolved Hide resolved
gora-benchmark/pom.xml Outdated Show resolved Hide resolved
<groupId>com.yahoo.ycsb</groupId>
<artifactId>core</artifactId>
<version>0.1.4</version>
</dependency>

This comment has been minimized.

Copy link
@lewismc

lewismc Aug 10, 2019

Contributor

If this dependency is not available from Maven central then we need to address that. How are you currently fetching it?

This comment has been minimized.

Copy link
@sneceesay77

sneceesay77 Aug 10, 2019

Author Contributor

I understand this isn't the standard approach but what I did was to download the jar file manually from maven website and place it in ~/.m2/repository/com/yahoo/ycsb/core/0.1.4/. I saw this trick from [1]. Also [2] and [3] experienced similar problems.

I will log an issue on YCSB GitHub page.

[1] https://stackoverflow.com/questions/22600408/use-maven-to-compile-ycsb
[2] blockchain-jd-com/jdchain#13
[3] brianfrankcooper/YCSB#174

This comment has been minimized.

Copy link
@sneceesay77

sneceesay77 Aug 10, 2019

Author Contributor

I have got some feedback from YCSB maintainers [1]. So basically, they have not published ycsb in maven central.

[1] brianfrankcooper/YCSB#1340

This comment has been minimized.

Copy link
@lewismc

lewismc Aug 12, 2019

Contributor

We need to help them on that issue. I will work with them to do that. Hopefully when they release 0.17.0 we can upgrade here.

gora-tutorial/conf/gora.properties Outdated Show resolved Hide resolved
@sneceesay77

This comment has been minimized.

Copy link
Contributor Author

sneceesay77 commented Aug 10, 2019

@lewismc, so what's the way forward regarding the YCSB dependency issue? I have updated my local dependency to the latest version of YCSB i.e. 0.15.0 and updated my code accordingly. I have added these changes to this pull request.

Right now the simplest way to go around this without maven is to:

  1. Download YCSB from [1] and copy all the jar files in ycsb-0.15.0/lib directory to gora-benchmark/lib. I have tested it and it is working with both MongoDB and HBase.
  2. Copy ycsb-0.15.0/lib/core-0.15.0.jar to ~/.m2/repository/com/yahoo/ycsb/core/0.15.0/
  3. mvn clean install

[1] https://github.com/brianfrankcooper/YCSB/releases/download/0.15.0/ycsb-0.15.0.tar.gz

@djkevincr, I have updated your comments in the latest commit.

Copy link
Contributor

lewismc left a comment

@sneceesay77 thanks again for updating this has again improved.
Please address my comments. In the meantime I will start testing locally and then I will follow up with you.
Excellent job.

@@ -0,0 +1,26 @@
<?xml version="1.0" encoding="UTF-8" standalone="no"?>

This comment has been minimized.

Copy link
@lewismc

lewismc Aug 19, 2019

Contributor

please add it here then

pom.xml Outdated Show resolved Hide resolved
@kamaci

This comment has been minimized.

Copy link
Member

kamaci commented Aug 20, 2019

Thanks for the effort you made! Could you check indentation throughout the PR files?

@sneceesay77 sneceesay77 reopened this Aug 20, 2019
@djkevincr djkevincr changed the title GORA-532: Apache Gora Benchmark initial pull request for review and comments GORA-532: Apache Gora Benchmark Module Aug 22, 2019
gora-benchmark/README.md Outdated Show resolved Hide resolved
@lewismc

This comment has been minimized.

Copy link
Contributor

lewismc commented Sep 6, 2019

@sneceesay77 can you please address all updated comments and then resolve the conflict in pom.xml. For the time being also please comment out this module if it does not compile without the dependency we are waiting on. We can come back to it later on and re-enable it. Thank you

Copy link
Contributor Author

sneceesay77 left a comment

This is all done now.

@sneceesay77

This comment has been minimized.

Copy link
Contributor Author

sneceesay77 commented Sep 7, 2019

@sneceesay77 can you please address all updated comments and then resolve the conflict in pom.xml. For the time being also please comment out this module if it does not compile without the dependency we are waiting on. We can come back to it later on and re-enable it. Thank you

@lewismc and @djkevincr , I have addressed all the comments and commented out the gora-benchmark module from the list of modules. Can you please have a look and please let me know if the changes requested are satisfied?

@lewismc

This comment has been minimized.

Copy link
Contributor

lewismc commented Sep 8, 2019

OK @sneceesay77 we've criticized this patch pretty well... you've done an excellent job so I'm going to go ahead and merge. Good job.
Please let us know when the YCSB dependency artifact is available and update this module with the dependency specifics at that point in time. Thank you.

@lewismc lewismc merged commit a354b35 into apache:master Sep 8, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
4 participants
You can’t perform that action at this time.