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
[CARBONDATA-1899] Add CarbonData concurrency test case #1670
Conversation
Build Failed with Spark 2.1.0, Please check CI http://136.243.101.176:8080/job/ApacheCarbonPRBuilder1/2014/ |
Build Failed with Spark 2.2.0, Please check CI http://88.99.58.216:8080/job/ApacheCarbonPRBuilder/794/ |
SDV Build Success , Please check CI http://144.76.159.231:8080/job/ApacheSDVTests/2337/ |
* limitations under the License. | ||
*/ | ||
|
||
package org.apache.carbondata.spark.testsuite.dataretention |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You do not need to add in testcase, I think you can add it in a separated benchmark module
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do you mean add it into example module?
7de9899
to
8618c3e
Compare
Build Failed with Spark 2.1.0, Please check CI http://136.243.101.176:8080/job/ApacheCarbonPRBuilder1/2074/ |
8618c3e
to
08c885f
Compare
SDV Build Success , Please check CI http://144.76.159.231:8080/job/ApacheSDVTests/2375/ |
Build Failed with Spark 2.2.0, Please check CI http://88.99.58.216:8080/job/ApacheCarbonPRBuilder/849/ |
Build Success with Spark 2.2.0, Please check CI http://88.99.58.216:8080/job/ApacheCarbonPRBuilder/851/ |
Build Success with Spark 2.1.0, Please check CI http://136.243.101.176:8080/job/ApacheCarbonPRBuilder1/2076/ |
SDV Build Success , Please check CI http://144.76.159.231:8080/job/ApacheSDVTests/2377/ |
SDV Build Success , Please check CI http://144.76.159.231:8080/job/ApacheSDVTests/2378/ |
08c885f
to
054a623
Compare
Build Success with Spark 2.2.0, Please check CI http://88.99.58.216:8080/job/ApacheCarbonPRBuilder/862/ |
Build Success with Spark 2.1.0, Please check CI http://136.243.101.176:8080/job/ApacheCarbonPRBuilder1/2087/ |
SDV Build Fail , Please check CI http://144.76.159.231:8080/job/ApacheSDVTests/2385/ |
retest this please |
val executorService = Executors.newFixedThreadPool(ThreadNum) | ||
val results = new util.ArrayList[Future[Results]]() | ||
for (num <- (1 to TaskNum).par) { | ||
results.add(executorService.submit(new QueryTask(spark, sqlText))) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
For better concurrency, I think invokeAll is better option instead of submitting task every time
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ok, I am testing the invokeALl in local
Build Success with Spark 2.2.0, Please check CI http://88.99.58.216:8080/job/ApacheCarbonPRBuilder/872/ |
Build Success with Spark 2.1.0, Please check CI http://136.243.101.176:8080/job/ApacheCarbonPRBuilder1/2097/ |
054a623
to
213c964
Compare
Build Success with Spark 2.1.0, Please check CI http://136.243.101.176:8080/job/ApacheCarbonPRBuilder1/2135/ |
Build Success with Spark 2.2.0, Please check CI http://88.99.58.216:8080/job/ApacheCarbonPRBuilder/911/ |
SDV Build Fail , Please check CI http://144.76.159.231:8080/job/ApacheSDVTests/2424/ |
retest sdv please |
val sql = s"query ${index + 1}: $sqlText " | ||
printResult(results, sql) | ||
executorService.shutdown() | ||
executorService.awaitTermination(600, TimeUnit.SECONDS) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
why printResult is getting called before shutting down executorService?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Because printResult invoke get() method(java.util.concurrent.Future#get()), it waits if necessary for the computation to complete, and then retrieves its result. So the tasks have finished when shutting down executorService
val r = new Random() | ||
val tmpId = r.nextInt(cardinalityId) % totalNum | ||
val tmpCity = "city" + (r.nextInt(cardinalityCity) % totalNum) | ||
val queries: Array[Query] = Array( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think it is better to give an option to read queries, table name and database name from external file...so that any number of queries can be added without modifying the code.....This will also help to standalone run this class and specifying the query file path as an argument to it.
If not now, you can enhance the framework in another PR
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
OK, I will enhance the framework.
.getOrCreateCarbonSession(storeLocation) | ||
spark.sparkContext.setLogLevel("warn") | ||
|
||
doParameter(args) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Change the method name to initParameters
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ok, done
val sortTimeArray = timeArray.sorted | ||
|
||
val time90 = ((sortTimeArray.length) * 0.9).toInt - 1 | ||
val time99 = ((sortTimeArray.length) * 0.99).toInt - 1 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please add a detailed comment for your printing logic and explain what is time90 and time99
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
done
SDV Build Success , Please check CI http://144.76.159.231:8080/job/ApacheSDVTests/2445/ |
Build Success with Spark 2.2.0, Please check CI http://88.99.58.216:8080/job/ApacheCarbonPRBuilder/938/ |
Build Success with Spark 2.1.0, Please check CI http://136.243.101.176:8080/job/ApacheCarbonPRBuilder1/2167/ |
75f5432
to
8116b3a
Compare
LGTM...once build runs I will merge |
retest this please |
Build Failed with Spark 2.2.0, Please check CI http://88.99.58.216:8080/job/ApacheCarbonPRBuilder/973/ |
Build Failed with Spark 2.1.0, Please check CI http://136.243.101.176:8080/job/ApacheCarbonPRBuilder1/2196/ |
Add CarbonData concurrency test case move test to excample module add doParameter fix scala style change the code by review comment
8116b3a
to
dd4f3dc
Compare
Build Success with Spark 2.2.0, Please check CI http://88.99.58.216:8080/job/ApacheCarbonPRBuilder/977/ |
Build Failed with Spark 2.1.0, Please check CI http://136.243.101.176:8080/job/ApacheCarbonPRBuilder1/2201/ |
LGTM |
Add CarbonData concurrency test case This closes apache#1670
Add CarbonData concurrency test case This closes apache#1670
Add CarbonData concurrency test case This closes apache#1670
Add CarbonData concurrency test case
Be sure to do all of the following checklist to help us incorporate
your contribution quickly and easily:
Any interfaces changed?
No
Any backward compatibility impacted?
No
Document update required?
No
Testing done
Please provide details on
- Whether new unit test cases have been added or why no new tests are required?
- How it is tested? Please attach test report.
- Is it a performance related change? Please attach the performance test report.
- Any additional information to help reviewers in testing this change.
For large changes, please consider breaking it into sub-tasks under an umbrella JIRA.
No