You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@carbondata.apache.org by ManoharVanam <gi...@git.apache.org> on 2017/12/05 13:17:39 UTC
[GitHub] carbondata pull request #1614: [CARBONDATA-1592] Changed Event params
GitHub user ManoharVanam opened a pull request:
https://github.com/apache/carbondata/pull/1614
[CARBONDATA-1592] Changed Event params
Description :
1. Added AlterTableCompactionPreStatusUpdateEvent
2. Changed compaction Event params
Be sure to do all of the following checklist to help us incorporate
your contribution quickly and easily:
- [ ] Any interfaces changed?
- [ ] Any backward compatibility impacted?
- [ ] Document update required?
- [ ] 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.
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/ManoharVanam/incubator-carbondata Acl2
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/carbondata/pull/1614.patch
To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:
This closes #1614
----
commit 27b920e9e546081576602a2f6b136e9200c76dad
Author: Manohar <ma...@gmail.com>
Date: 2017-12-05T13:14:29Z
Changed Compaction Event params
Added AlterTableCompactionPreStatusUpdateEvent
----
---
[GitHub] carbondata issue #1614: [CARBONDATA-1592] Changed Event params
Posted by CarbonDataQA <gi...@git.apache.org>.
Github user CarbonDataQA commented on the issue:
https://github.com/apache/carbondata/pull/1614
Build Success with Spark 2.1.0, Please check CI http://136.243.101.176:8080/job/ApacheCarbonPRBuilder1/1728/
---
[GitHub] carbondata issue #1614: [CARBONDATA-1592] Changed Event params
Posted by jackylk <gi...@git.apache.org>.
Github user jackylk commented on the issue:
https://github.com/apache/carbondata/pull/1614
Please update the PR title and description to include more information
---
[GitHub] carbondata issue #1614: [CARBONDATA-1592] Added Event Listeners & Removed Un...
Posted by CarbonDataQA <gi...@git.apache.org>.
Github user CarbonDataQA commented on the issue:
https://github.com/apache/carbondata/pull/1614
Build Success with Spark 2.2.0, Please check CI http://88.99.58.216:8080/job/ApacheCarbonPRBuilder/580/
---
[GitHub] carbondata issue #1614: [CARBONDATA-1592] Added Event Listeners & Removed Un...
Posted by CarbonDataQA <gi...@git.apache.org>.
Github user CarbonDataQA commented on the issue:
https://github.com/apache/carbondata/pull/1614
Build Success with Spark 2.2.0, Please check CI http://88.99.58.216:8080/job/ApacheCarbonPRBuilder/585/
---
[GitHub] carbondata pull request #1614: [CARBONDATA-1592] Added Event Listeners & Rem...
Posted by ManoharVanam <gi...@git.apache.org>.
Github user ManoharVanam commented on a diff in the pull request:
https://github.com/apache/carbondata/pull/1614#discussion_r155157176
--- Diff: integration/spark-common/src/main/scala/org/apache/carbondata/events/Events.scala ---
@@ -93,12 +93,20 @@ trait AlterTableAddColumnEventInfo {
/**
* event for alter_table_rename
*/
-trait AlterTableCompactionEventInfo {
+trait AlterTableCompactionStatusUpdateEventInfo {
--- End diff --
+ loadsToMerge: util.List[LoadMetadataDetails],
+ compactionType: CompactionType,
+ sQLContext: SQLContext)
+ extends Event with AlterTableCompactionStatusUpdateEventInfo
---
[GitHub] carbondata pull request #1614: [CARBONDATA-1592] Added Event Listeners & Rem...
Posted by jackylk <gi...@git.apache.org>.
Github user jackylk commented on a diff in the pull request:
https://github.com/apache/carbondata/pull/1614#discussion_r155261703
--- Diff: integration/spark-common/src/main/scala/org/apache/carbondata/events/AlterTableEvents.scala ---
@@ -131,40 +135,44 @@ case class AlterTableRenameAbortEvent(carbonTable: CarbonTable,
/**
- *
+ * Class for handling pre compaction operations
--- End diff --
This is not the class to handle the event, please describe when this event will be triggered. And add description of how it is handled in the corresponding listener
---
[GitHub] carbondata pull request #1614: [CARBONDATA-1592] Changed Event params
Posted by ManoharVanam <gi...@git.apache.org>.
Github user ManoharVanam commented on a diff in the pull request:
https://github.com/apache/carbondata/pull/1614#discussion_r155156007
--- Diff: integration/spark-common/src/main/scala/org/apache/carbondata/events/AlterTableEvents.scala ---
@@ -132,39 +136,33 @@ case class AlterTableRenameAbortEvent(carbonTable: CarbonTable,
/**
*
+ * @param sparkSession
* @param carbonTable
- * @param carbonLoadModel
- * @param mergedLoadName
- * @param sQLContext
*/
-case class AlterTableCompactionPreEvent(sparkSession: SparkSession, carbonTable: CarbonTable,
- carbonLoadModel: CarbonLoadModel,
- mergedLoadName: String,
- sQLContext: SQLContext) extends Event with AlterTableCompactionEventInfo
-
+case class AlterTableCompactionPreEvent(sparkSession: SparkSession, carbonTable: CarbonTable)
--- End diff --
ok
---
[GitHub] carbondata issue #1614: [CARBONDATA-1592] Added Event Listeners & Removed Un...
Posted by CarbonDataQA <gi...@git.apache.org>.
Github user CarbonDataQA commented on the issue:
https://github.com/apache/carbondata/pull/1614
Build Success with Spark 2.2.0, Please check CI http://88.99.58.216:8080/job/ApacheCarbonPRBuilder/489/
---
[GitHub] carbondata issue #1614: [CARBONDATA-1592] Changed Event params
Posted by CarbonDataQA <gi...@git.apache.org>.
Github user CarbonDataQA commented on the issue:
https://github.com/apache/carbondata/pull/1614
Build Success with Spark 2.2.0, Please check CI http://88.99.58.216:8080/job/ApacheCarbonPRBuilder/461/
---
[GitHub] carbondata issue #1614: [CARBONDATA-1592] Added Event Listeners & Removed Un...
Posted by CarbonDataQA <gi...@git.apache.org>.
Github user CarbonDataQA commented on the issue:
https://github.com/apache/carbondata/pull/1614
Build Success with Spark 2.1.0, Please check CI http://136.243.101.176:8080/job/ApacheCarbonPRBuilder1/1753/
---
[GitHub] carbondata issue #1614: [CARBONDATA-1592] Added Event Listeners & Removed Un...
Posted by CarbonDataQA <gi...@git.apache.org>.
Github user CarbonDataQA commented on the issue:
https://github.com/apache/carbondata/pull/1614
Build Success with Spark 2.1.0, Please check CI http://136.243.101.176:8080/job/ApacheCarbonPRBuilder1/1830/
---
[GitHub] carbondata pull request #1614: [CARBONDATA-1592] Added Event Listeners & Rem...
Posted by asfgit <gi...@git.apache.org>.
Github user asfgit closed the pull request at:
https://github.com/apache/carbondata/pull/1614
---
[GitHub] carbondata issue #1614: [CARBONDATA-1592] Added Event Listeners & Removed Un...
Posted by CarbonDataQA <gi...@git.apache.org>.
Github user CarbonDataQA commented on the issue:
https://github.com/apache/carbondata/pull/1614
Build Failed with Spark 2.1.0, Please check CI http://136.243.101.176:8080/job/ApacheCarbonPRBuilder1/1823/
---
[GitHub] carbondata issue #1614: [CARBONDATA-1592] Added Event Listeners & Removed Un...
Posted by CarbonDataQA <gi...@git.apache.org>.
Github user CarbonDataQA commented on the issue:
https://github.com/apache/carbondata/pull/1614
Build Success with Spark 2.1.0, Please check CI http://136.243.101.176:8080/job/ApacheCarbonPRBuilder1/1833/
---
[GitHub] carbondata pull request #1614: [CARBONDATA-1592] Added Event Listeners & Rem...
Posted by ManoharVanam <gi...@git.apache.org>.
Github user ManoharVanam commented on a diff in the pull request:
https://github.com/apache/carbondata/pull/1614#discussion_r155414323
--- Diff: integration/spark-common/src/main/scala/org/apache/carbondata/events/Events.scala ---
@@ -93,12 +93,20 @@ trait AlterTableAddColumnEventInfo {
/**
* event for alter_table_rename
*/
-trait AlterTableCompactionEventInfo {
+trait AlterTableCompactionStatusUpdateEventInfo {
val carbonTable: CarbonTable
val carbonLoadModel: CarbonLoadModel
val mergedLoadName: String
}
+/**
+ * event for alter_table_compaction
+ */
+trait AlterTableCompactionEventInfo {
--- End diff --
This trait we are using in both pre & post compaction events , these two events are required for decouple purpose
---
[GitHub] carbondata pull request #1614: [CARBONDATA-1592] Added Event Listeners & Rem...
Posted by ManoharVanam <gi...@git.apache.org>.
Github user ManoharVanam commented on a diff in the pull request:
https://github.com/apache/carbondata/pull/1614#discussion_r155468332
--- Diff: integration/spark-common/src/main/scala/org/apache/carbondata/events/AlterTableEvents.scala ---
@@ -131,40 +135,49 @@ case class AlterTableRenameAbortEvent(carbonTable: CarbonTable,
/**
+ * Event for handling pre compaction operations, lister has to implement this event on pre execution
*
+ * @param sparkSession
* @param carbonTable
- * @param carbonLoadModel
- * @param mergedLoadName
- * @param sQLContext
*/
-case class AlterTableCompactionPreEvent(sparkSession: SparkSession, carbonTable: CarbonTable,
- carbonLoadModel: CarbonLoadModel,
- mergedLoadName: String,
- sQLContext: SQLContext) extends Event with AlterTableCompactionEventInfo
+case class AlterTableCompactionPreEvent(sparkSession: SparkSession, carbonTable: CarbonTable)
--- End diff --
ok
---
[GitHub] carbondata pull request #1614: [CARBONDATA-1592] Added Event Listeners & Rem...
Posted by gvramana <gi...@git.apache.org>.
Github user gvramana commented on a diff in the pull request:
https://github.com/apache/carbondata/pull/1614#discussion_r155442248
--- Diff: integration/spark-common/src/main/scala/org/apache/carbondata/events/AlterTableEvents.scala ---
@@ -131,40 +135,49 @@ case class AlterTableRenameAbortEvent(carbonTable: CarbonTable,
/**
+ * Event for handling pre compaction operations, lister has to implement this event on pre execution
*
+ * @param sparkSession
* @param carbonTable
- * @param carbonLoadModel
- * @param mergedLoadName
- * @param sQLContext
*/
-case class AlterTableCompactionPreEvent(sparkSession: SparkSession, carbonTable: CarbonTable,
- carbonLoadModel: CarbonLoadModel,
- mergedLoadName: String,
- sQLContext: SQLContext) extends Event with AlterTableCompactionEventInfo
+case class AlterTableCompactionPreEvent(sparkSession: SparkSession, carbonTable: CarbonTable)
--- End diff --
Add LoadTablePreStatusEvent Also
---
[GitHub] carbondata pull request #1614: [CARBONDATA-1592] Changed Event params
Posted by jackylk <gi...@git.apache.org>.
Github user jackylk commented on a diff in the pull request:
https://github.com/apache/carbondata/pull/1614#discussion_r154978196
--- Diff: integration/spark-common/src/main/scala/org/apache/carbondata/events/AlterTableEvents.scala ---
@@ -132,39 +136,33 @@ case class AlterTableRenameAbortEvent(carbonTable: CarbonTable,
/**
*
+ * @param sparkSession
* @param carbonTable
- * @param carbonLoadModel
- * @param mergedLoadName
- * @param sQLContext
*/
-case class AlterTableCompactionPreEvent(sparkSession: SparkSession, carbonTable: CarbonTable,
- carbonLoadModel: CarbonLoadModel,
- mergedLoadName: String,
- sQLContext: SQLContext) extends Event with AlterTableCompactionEventInfo
-
+case class AlterTableCompactionPreEvent(sparkSession: SparkSession, carbonTable: CarbonTable)
--- End diff --
Add description of each event, describe when it will be triggered
---
[GitHub] carbondata issue #1614: [CARBONDATA-1592] Added Event Listeners & Removed Un...
Posted by ravipesala <gi...@git.apache.org>.
Github user ravipesala commented on the issue:
https://github.com/apache/carbondata/pull/1614
SDV Build Success , Please check CI http://144.76.159.231:8080/job/ApacheSDVTests/2136/
---
[GitHub] carbondata issue #1614: [CARBONDATA-1592] Added Event Listeners & Removed Un...
Posted by CarbonDataQA <gi...@git.apache.org>.
Github user CarbonDataQA commented on the issue:
https://github.com/apache/carbondata/pull/1614
Build Success with Spark 2.2.0, Please check CI http://88.99.58.216:8080/job/ApacheCarbonPRBuilder/540/
---
[GitHub] carbondata issue #1614: [CARBONDATA-1592] Added Event Listeners & Removed Un...
Posted by gvramana <gi...@git.apache.org>.
Github user gvramana commented on the issue:
https://github.com/apache/carbondata/pull/1614
LGTM
---
[GitHub] carbondata pull request #1614: [CARBONDATA-1592] Changed Event params
Posted by jackylk <gi...@git.apache.org>.
Github user jackylk commented on a diff in the pull request:
https://github.com/apache/carbondata/pull/1614#discussion_r154978937
--- Diff: integration/spark-common/src/main/scala/org/apache/carbondata/events/Events.scala ---
@@ -93,12 +93,20 @@ trait AlterTableAddColumnEventInfo {
/**
* event for alter_table_rename
*/
-trait AlterTableCompactionEventInfo {
+trait AlterTableCompactionStatusUpdateEventInfo {
val carbonTable: CarbonTable
val carbonLoadModel: CarbonLoadModel
val mergedLoadName: String
}
+/**
+ * event for alter_table_compaction
+ */
+trait AlterTableCompactionEventInfo {
--- End diff --
Seems no where this is used
---
[GitHub] carbondata pull request #1614: [CARBONDATA-1592] Changed Event params
Posted by ManoharVanam <gi...@git.apache.org>.
Github user ManoharVanam commented on a diff in the pull request:
https://github.com/apache/carbondata/pull/1614#discussion_r155155665
--- Diff: integration/spark-common/src/main/scala/org/apache/carbondata/events/Events.scala ---
@@ -93,12 +93,20 @@ trait AlterTableAddColumnEventInfo {
/**
* event for alter_table_rename
*/
-trait AlterTableCompactionEventInfo {
+trait AlterTableCompactionStatusUpdateEventInfo {
--- End diff --
This will be used in SI listener
---
[GitHub] carbondata issue #1614: [CARBONDATA-1592] Added Event Listeners & Removed Un...
Posted by CarbonDataQA <gi...@git.apache.org>.
Github user CarbonDataQA commented on the issue:
https://github.com/apache/carbondata/pull/1614
Build Success with Spark 2.2.0, Please check CI http://88.99.58.216:8080/job/ApacheCarbonPRBuilder/567/
---
[GitHub] carbondata pull request #1614: [CARBONDATA-1592] Added Event Listeners & Rem...
Posted by ManoharVanam <gi...@git.apache.org>.
Github user ManoharVanam commented on a diff in the pull request:
https://github.com/apache/carbondata/pull/1614#discussion_r155518002
--- Diff: integration/spark2/src/main/scala/org/apache/carbondata/spark/rdd/CarbonDataRDDFactory.scala ---
@@ -492,6 +492,12 @@ object CarbonDataRDDFactory {
throw new Exception("No Data to load")
}
writeDictionary(carbonLoadModel, result, writeAll = false)
+ val loadTablePreStatusUpdateEvent: LoadTablePreStatusUpdateEvent =
--- End diff --
done
---
[GitHub] carbondata issue #1614: [CARBONDATA-1592] Added Event Listeners & Removed Un...
Posted by CarbonDataQA <gi...@git.apache.org>.
Github user CarbonDataQA commented on the issue:
https://github.com/apache/carbondata/pull/1614
Build Success with Spark 2.1.0, Please check CI http://136.243.101.176:8080/job/ApacheCarbonPRBuilder1/1797/
---
[GitHub] carbondata issue #1614: [CARBONDATA-1592] Changed Event params
Posted by ravipesala <gi...@git.apache.org>.
Github user ravipesala commented on the issue:
https://github.com/apache/carbondata/pull/1614
SDV Build Fail , Please check CI http://144.76.159.231:8080/job/ApacheSDVTests/2110/
---
[GitHub] carbondata issue #1614: [CARBONDATA-1592] Added Event Listeners & Removed Un...
Posted by CarbonDataQA <gi...@git.apache.org>.
Github user CarbonDataQA commented on the issue:
https://github.com/apache/carbondata/pull/1614
Build Success with Spark 2.1.0, Please check CI http://136.243.101.176:8080/job/ApacheCarbonPRBuilder1/1815/
---
[GitHub] carbondata issue #1614: [CARBONDATA-1592] Added Event Listeners & Removed Un...
Posted by CarbonDataQA <gi...@git.apache.org>.
Github user CarbonDataQA commented on the issue:
https://github.com/apache/carbondata/pull/1614
Build Success with Spark 2.2.0, Please check CI http://88.99.58.216:8080/job/ApacheCarbonPRBuilder/602/
---
[GitHub] carbondata pull request #1614: [CARBONDATA-1592] Added Event Listeners & Rem...
Posted by ManoharVanam <gi...@git.apache.org>.
Github user ManoharVanam commented on a diff in the pull request:
https://github.com/apache/carbondata/pull/1614#discussion_r155414167
--- Diff: integration/spark-common/src/main/scala/org/apache/carbondata/events/AlterTableEvents.scala ---
@@ -131,40 +135,44 @@ case class AlterTableRenameAbortEvent(carbonTable: CarbonTable,
/**
- *
+ * Class for handling pre compaction operations
--- End diff --
ok
---
[GitHub] carbondata issue #1614: [CARBONDATA-1592] Added Event Listeners & Removed Un...
Posted by ravipesala <gi...@git.apache.org>.
Github user ravipesala commented on the issue:
https://github.com/apache/carbondata/pull/1614
SDV Build Success , Please check CI http://144.76.159.231:8080/job/ApacheSDVTests/2200/
---
[GitHub] carbondata issue #1614: [CARBONDATA-1592] Added Event Listeners & Removed Un...
Posted by ravipesala <gi...@git.apache.org>.
Github user ravipesala commented on the issue:
https://github.com/apache/carbondata/pull/1614
SDV Build Fail , Please check CI http://144.76.159.231:8080/job/ApacheSDVTests/2171/
---
[GitHub] carbondata pull request #1614: [CARBONDATA-1592] Added Event Listeners & Rem...
Posted by gvramana <gi...@git.apache.org>.
Github user gvramana commented on a diff in the pull request:
https://github.com/apache/carbondata/pull/1614#discussion_r155496669
--- Diff: integration/spark2/src/main/scala/org/apache/carbondata/spark/rdd/CarbonDataRDDFactory.scala ---
@@ -492,6 +492,12 @@ object CarbonDataRDDFactory {
throw new Exception("No Data to load")
}
writeDictionary(carbonLoadModel, result, writeAll = false)
+ val loadTablePreStatusUpdateEvent: LoadTablePreStatusUpdateEvent =
--- End diff --
Trigger should pass operationContext
---
[GitHub] carbondata pull request #1614: [CARBONDATA-1592] Added Event Listeners & Rem...
Posted by jackylk <gi...@git.apache.org>.
Github user jackylk commented on a diff in the pull request:
https://github.com/apache/carbondata/pull/1614#discussion_r155261399
--- Diff: integration/spark-common/src/main/scala/org/apache/carbondata/events/Events.scala ---
@@ -93,12 +93,20 @@ trait AlterTableAddColumnEventInfo {
/**
* event for alter_table_rename
*/
-trait AlterTableCompactionEventInfo {
+trait AlterTableCompactionStatusUpdateEventInfo {
val carbonTable: CarbonTable
val carbonLoadModel: CarbonLoadModel
val mergedLoadName: String
}
+/**
+ * event for alter_table_compaction
+ */
+trait AlterTableCompactionEventInfo {
--- End diff --
AlterTableCompactionPostEvent has sparkSession and carbonTable already, why this trait is required
---
[GitHub] carbondata issue #1614: [CARBONDATA-1592] Added Event Listeners & Removed Un...
Posted by CarbonDataQA <gi...@git.apache.org>.
Github user CarbonDataQA commented on the issue:
https://github.com/apache/carbondata/pull/1614
Build Failed with Spark 2.2.0, Please check CI http://88.99.58.216:8080/job/ApacheCarbonPRBuilder/605/
---
[GitHub] carbondata pull request #1614: [CARBONDATA-1592] Changed Event params
Posted by ManoharVanam <gi...@git.apache.org>.
Github user ManoharVanam commented on a diff in the pull request:
https://github.com/apache/carbondata/pull/1614#discussion_r155155904
--- Diff: integration/spark-common/src/main/scala/org/apache/carbondata/events/Events.scala ---
@@ -93,12 +93,20 @@ trait AlterTableAddColumnEventInfo {
/**
* event for alter_table_rename
*/
-trait AlterTableCompactionEventInfo {
+trait AlterTableCompactionStatusUpdateEventInfo {
val carbonTable: CarbonTable
val carbonLoadModel: CarbonLoadModel
val mergedLoadName: String
}
+/**
+ * event for alter_table_compaction
+ */
+trait AlterTableCompactionEventInfo {
--- End diff --
+case class AlterTableCompactionPostEvent(sparkSession: SparkSession, carbonTable: CarbonTable)
+ extends Event with AlterTableCompactionEventInfo
---
[GitHub] carbondata issue #1614: [CARBONDATA-1592] Added Event Listeners & Removed Un...
Posted by gvramana <gi...@git.apache.org>.
Github user gvramana commented on the issue:
https://github.com/apache/carbondata/pull/1614
LGTM
---
[GitHub] carbondata pull request #1614: [CARBONDATA-1592] Changed Event params
Posted by jackylk <gi...@git.apache.org>.
Github user jackylk commented on a diff in the pull request:
https://github.com/apache/carbondata/pull/1614#discussion_r154978890
--- Diff: integration/spark-common/src/main/scala/org/apache/carbondata/events/Events.scala ---
@@ -93,12 +93,20 @@ trait AlterTableAddColumnEventInfo {
/**
* event for alter_table_rename
*/
-trait AlterTableCompactionEventInfo {
+trait AlterTableCompactionStatusUpdateEventInfo {
--- End diff --
Seems no where this is used
---