You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@carbondata.apache.org by GitBox <gi...@apache.org> on 2021/01/12 08:40:44 UTC
[GitHub] [carbondata] Indhumathi27 opened a new pull request #4076: [WIP] Added mvExists property in MV fact table and lock while touchMDTFile
Indhumathi27 opened a new pull request #4076:
URL: https://github.com/apache/carbondata/pull/4076
### Why is this PR needed?
### What changes were proposed in this PR?
### Does this PR introduce any user interface change?
- No
- Yes. (please explain the change and update document)
### Is any new testcase added?
- No
- Yes
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [carbondata] CarbonDataQA2 commented on pull request #4076: [CARBONDATA-4107] Added related MV tables Map to fact table and added lock while touchMDTFile
Posted by GitBox <gi...@apache.org>.
CarbonDataQA2 commented on pull request #4076:
URL: https://github.com/apache/carbondata/pull/4076#issuecomment-769714541
Build Success with Spark 2.3.4, Please check CI http://121.244.95.60:12444/job/ApacheCarbonPRBuilder2.3/5377/
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [carbondata] akashrn5 commented on pull request #4076: [CARBONDATA-4107] Added related MV tables Map to fact table and added lock while touchMDTFile
Posted by GitBox <gi...@apache.org>.
akashrn5 commented on pull request #4076:
URL: https://github.com/apache/carbondata/pull/4076#issuecomment-773160807
LGTM
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [carbondata] CarbonDataQA2 commented on pull request #4076: [CARBONDATA-4107] Added mvExists property for MV fact table and added lock while touchMDTFile
Posted by GitBox <gi...@apache.org>.
CarbonDataQA2 commented on pull request #4076:
URL: https://github.com/apache/carbondata/pull/4076#issuecomment-759457085
Build Success with Spark 2.4.5, Please check CI http://121.244.95.60:12444/job/ApacheCarbon_PR_Builder_2.4.5/3548/
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [carbondata] akashrn5 commented on a change in pull request #4076: [CARBONDATA-4107] Added related MV tables Map to fact table and added lock while touchMDTFile
Posted by GitBox <gi...@apache.org>.
akashrn5 commented on a change in pull request #4076:
URL: https://github.com/apache/carbondata/pull/4076#discussion_r569996310
##########
File path: docs/mv-guide.md
##########
@@ -1,3 +1,4 @@
+
Review comment:
remove this empty line change
##########
File path: docs/mv-guide.md
##########
@@ -241,6 +242,11 @@ The current information includes:
| Refresh Mode | FULL / INCREMENTAL refresh to MV |
| Refresh Trigger Mode | ON_COMMIT / ON_MANUAL refresh to MV provided by user |
| Properties | Table properties of the materialized view |
+
+**NOTE**: For materialized views created
+before [CARBONDATA-4107](https://issues.apache.org/jira/browse/CARBONDATA-4107) issue fix, run
+refresh mv command to add mv name to fact table property and to enable it. If refresh command
Review comment:
```suggestion
refresh mv command to add mv name to fact table's table properties and to enable it. If refresh command
```
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [carbondata] akashrn5 commented on a change in pull request #4076: [CARBONDATA-4107] Added related MV tables Map to fact table and added lock while touchMDTFile
Posted by GitBox <gi...@apache.org>.
akashrn5 commented on a change in pull request #4076:
URL: https://github.com/apache/carbondata/pull/4076#discussion_r568501151
##########
File path: docs/mv-guide.md
##########
@@ -241,6 +242,10 @@ The current information includes:
| Refresh Mode | FULL / INCREMENTAL refresh to MV |
| Refresh Trigger Mode | ON_COMMIT / ON_MANUAL refresh to MV provided by user |
| Properties | Table properties of the materialized view |
+
+**NOTE**: For materialized views created
+before [CARBONDATA-4107](https://issues.apache.org/jira/browse/CARBONDATA-4107) issue fix, run
+refresh mv command to add mv name to fact table property and to enable it.
Review comment:
also please add what happens if user doesn't use refresh
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [carbondata] akashrn5 commented on a change in pull request #4076: [CARBONDATA-4107] Added related MV tables Map to fact table and added lock while touchMDTFile
Posted by GitBox <gi...@apache.org>.
akashrn5 commented on a change in pull request #4076:
URL: https://github.com/apache/carbondata/pull/4076#discussion_r569996310
##########
File path: docs/mv-guide.md
##########
@@ -1,3 +1,4 @@
+
Review comment:
remove this empty line change
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [carbondata] CarbonDataQA2 commented on pull request #4076: [CARBONDATA-4107] Added related MV tables Map to fact table and added lock while touchMDTFile
Posted by GitBox <gi...@apache.org>.
CarbonDataQA2 commented on pull request #4076:
URL: https://github.com/apache/carbondata/pull/4076#issuecomment-771647871
Build Success with Spark 2.4.5, Please check CI http://121.244.95.60:12444/job/ApacheCarbon_PR_Builder_2.4.5/3651/
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [carbondata] QiangCai commented on pull request #4076: [WIP][CARBONDATA-4107] Added mvExists property for MV fact table and added lock while touchMDTFile
Posted by GitBox <gi...@apache.org>.
QiangCai commented on pull request #4076:
URL: https://github.com/apache/carbondata/pull/4076#issuecomment-760627378
how about to list all mv table in fact table properties?
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [carbondata] Indhumathi27 commented on pull request #4076: [WIP][CARBONDATA-4107] Added mvExists property for MV fact table and added lock while touchMDTFile
Posted by GitBox <gi...@apache.org>.
Indhumathi27 commented on pull request #4076:
URL: https://github.com/apache/carbondata/pull/4076#issuecomment-760646765
@QiangCai Even if we list all mv tables in fact table, we need the database information in order to fetch schema files. and also, a table can have more number of MV's, which can increase size of fact table
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [carbondata] CarbonDataQA2 commented on pull request #4076: [CARBONDATA-4107] Added related MV tables Map to fact table and added lock while touchMDTFile
Posted by GitBox <gi...@apache.org>.
CarbonDataQA2 commented on pull request #4076:
URL: https://github.com/apache/carbondata/pull/4076#issuecomment-766553406
Build Failed with Spark 2.4.5, Please check CI http://121.244.95.60:12545/job/ApacheCarbon_PR_Builder_2.4.5/3291/
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [carbondata] CarbonDataQA2 commented on pull request #4076: [CARBONDATA-4107] Added related MV tables Map to fact table and added lock while touchMDTFile
Posted by GitBox <gi...@apache.org>.
CarbonDataQA2 commented on pull request #4076:
URL: https://github.com/apache/carbondata/pull/4076#issuecomment-766795669
Build Success with Spark 2.4.5, Please check CI http://121.244.95.60:12444/job/ApacheCarbon_PR_Builder_2.4.5/3593/
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [carbondata] CarbonDataQA2 commented on pull request #4076: [CARBONDATA-4107] Added related MV tables Map to fact table and added lock while touchMDTFile
Posted by GitBox <gi...@apache.org>.
CarbonDataQA2 commented on pull request #4076:
URL: https://github.com/apache/carbondata/pull/4076#issuecomment-765429542
Build Failed with Spark 2.4.5, Please check CI http://121.244.95.60:12444/job/ApacheCarbon_PR_Builder_2.4.5/3578/
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [carbondata] CarbonDataQA2 commented on pull request #4076: [CARBONDATA-4107] Added related MV tables Map to fact table and added lock while touchMDTFile
Posted by GitBox <gi...@apache.org>.
CarbonDataQA2 commented on pull request #4076:
URL: https://github.com/apache/carbondata/pull/4076#issuecomment-768212353
Build Success with Spark 2.4.5, Please check CI http://121.244.95.60:12444/job/ApacheCarbon_PR_Builder_2.4.5/3601/
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [carbondata] Indhumathi27 commented on a change in pull request #4076: [CARBONDATA-4107] Added mvExists property for MV fact table and added lock while touchMDTFile
Posted by GitBox <gi...@apache.org>.
Indhumathi27 commented on a change in pull request #4076:
URL: https://github.com/apache/carbondata/pull/4076#discussion_r561693339
##########
File path: integration/spark/src/main/scala/org/apache/spark/sql/execution/command/view/CarbonDropMVCommand.scala
##########
@@ -90,6 +95,37 @@ case class CarbonDropMVCommand(
}
}
+ // Update the mvExists and related databases property to mv fact tables
+ schema.getRelatedTables.asScala.foreach { table =>
+ val dbName = table.getDatabaseName
+ val tableName = table.getTableName
+ try {
+ val carbonTable =
+ CarbonEnv.getCarbonTable(Some(dbName), tableName)(session)
+ val relatedMVTablesMap = carbonTable.getMVTablesMap
+ // check if database has materialized views or not
+ val anotherMVExistsInDb = viewManager.getSchemas(databaseName).asScala.exists {
+ mvSchema =>
+ mvSchema.getRelatedTables.asScala.exists(_.getTableName.equalsIgnoreCase(tableName))
+ }
+ if (!anotherMVExistsInDb) {
+ // If database dont have any MV, then remove the database from related tables
+ // property and update table property of fact table
+ relatedMVTablesMap.get(databaseName).remove(name)
+ if (relatedMVTablesMap.get(databaseName).isEmpty) {
+ relatedMVTablesMap.remove(databaseName)
+ }
+ CarbonIndexUtil.addOrModifyTableProperty(carbonTable,
+ Map("relatedMVTablesMap".toLowerCase -> new Gson().toJson(relatedMVTablesMap)),
+ needLock = false)(session)
+ CarbonHiveIndexMetadataUtil.refreshTable(dbName, tableName, session)
+ }
+ } catch {
+ case _: Exception =>
+ // ignore as table is a non-carbon table
Review comment:
This Exception is added, because for parquet and orc, carbon table will not be present.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [carbondata] Indhumathi27 commented on pull request #4076: [CARBONDATA-4107] Added related MV tables Map to fact table and added lock while touchMDTFile
Posted by GitBox <gi...@apache.org>.
Indhumathi27 commented on pull request #4076:
URL: https://github.com/apache/carbondata/pull/4076#issuecomment-766746411
retest this please
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [carbondata] CarbonDataQA2 commented on pull request #4076: [CARBONDATA-4107] Added related MV tables Map to fact table and added lock while touchMDTFile
Posted by GitBox <gi...@apache.org>.
CarbonDataQA2 commented on pull request #4076:
URL: https://github.com/apache/carbondata/pull/4076#issuecomment-765347008
Build Failed with Spark 2.3.4, Please check CI http://121.244.95.60:12444/job/ApacheCarbonPRBuilder2.3/5336/
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [carbondata] CarbonDataQA2 commented on pull request #4076: [WIP] Added mvExists property for MV fact table and added lock while touchMDTFile
Posted by GitBox <gi...@apache.org>.
CarbonDataQA2 commented on pull request #4076:
URL: https://github.com/apache/carbondata/pull/4076#issuecomment-758577309
Build Success with Spark 2.3.4, Please check CI http://121.244.95.60:12444/job/ApacheCarbonPRBuilder2.3/5303/
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [carbondata] QiangCai commented on pull request #4076: [CARBONDATA-4107] Added mvExists property for MV fact table and added lock while touchMDTFile
Posted by GitBox <gi...@apache.org>.
QiangCai commented on pull request #4076:
URL: https://github.com/apache/carbondata/pull/4076#issuecomment-762096880
@Indhumathi27 if the number of mv tables is less than 10 (configurable), maybe we can list all tables; if-else we can store database information only.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [carbondata] CarbonDataQA2 commented on pull request #4076: [CARBONDATA-4107] Added mvExists property for MV fact table and added lock while touchMDTFile
Posted by GitBox <gi...@apache.org>.
CarbonDataQA2 commented on pull request #4076:
URL: https://github.com/apache/carbondata/pull/4076#issuecomment-762670736
Build Failed with Spark 2.3.4, Please check CI http://121.244.95.60:12444/job/ApacheCarbonPRBuilder2.3/5314/
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [carbondata] Indhumathi27 commented on pull request #4076: [CARBONDATA-4107] Added related MV tables Map to fact table and added lock while touchMDTFile
Posted by GitBox <gi...@apache.org>.
Indhumathi27 commented on pull request #4076:
URL: https://github.com/apache/carbondata/pull/4076#issuecomment-772423833
retest this please
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [carbondata] CarbonDataQA2 commented on pull request #4076: [CARBONDATA-4107] Added related MV tables Map to fact table and added lock while touchMDTFile
Posted by GitBox <gi...@apache.org>.
CarbonDataQA2 commented on pull request #4076:
URL: https://github.com/apache/carbondata/pull/4076#issuecomment-765228520
Build Success with Spark 2.3.4, Please check CI http://121.244.95.60:12444/job/ApacheCarbonPRBuilder2.3/5334/
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [carbondata] CarbonDataQA2 commented on pull request #4076: [CARBONDATA-4107] Added mvExists property for MV fact table and added lock while touchMDTFile
Posted by GitBox <gi...@apache.org>.
CarbonDataQA2 commented on pull request #4076:
URL: https://github.com/apache/carbondata/pull/4076#issuecomment-764465842
Build Failed with Spark 2.3.4, Please check CI http://121.244.95.60:12444/job/ApacheCarbonPRBuilder2.3/5328/
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [carbondata] asfgit closed pull request #4076: [CARBONDATA-4107] Added related MV tables Map to fact table and added lock while touchMDTFile
Posted by GitBox <gi...@apache.org>.
asfgit closed pull request #4076:
URL: https://github.com/apache/carbondata/pull/4076
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [carbondata] Indhumathi27 commented on pull request #4076: [CARBONDATA-4107] Added related MV tables Map to fact table and added lock while touchMDTFile
Posted by GitBox <gi...@apache.org>.
Indhumathi27 commented on pull request #4076:
URL: https://github.com/apache/carbondata/pull/4076#issuecomment-765372067
retest this please
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [carbondata] CarbonDataQA2 commented on pull request #4076: [CARBONDATA-4107] Added mvExists property for MV fact table and added lock while touchMDTFile
Posted by GitBox <gi...@apache.org>.
CarbonDataQA2 commented on pull request #4076:
URL: https://github.com/apache/carbondata/pull/4076#issuecomment-764358350
Build Failed with Spark 2.3.4, Please check CI http://121.244.95.60:12444/job/ApacheCarbonPRBuilder2.3/5323/
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [carbondata] CarbonDataQA2 commented on pull request #4076: [WIP] Added mvExists property in MV fact table and lock while touchMDTFile
Posted by GitBox <gi...@apache.org>.
CarbonDataQA2 commented on pull request #4076:
URL: https://github.com/apache/carbondata/pull/4076#issuecomment-758502058
Build Failed with Spark 2.4.5, Please check CI http://121.244.95.60:12444/job/ApacheCarbon_PR_Builder_2.4.5/3541/
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [carbondata] Indhumathi27 commented on pull request #4076: [CARBONDATA-4107] Added mvExists property for MV fact table and added lock while touchMDTFile
Posted by GitBox <gi...@apache.org>.
Indhumathi27 commented on pull request #4076:
URL: https://github.com/apache/carbondata/pull/4076#issuecomment-764288658
retest this please
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [carbondata] Indhumathi27 commented on a change in pull request #4076: [CARBONDATA-4107] Added mvExists property for MV fact table and added lock while touchMDTFile
Posted by GitBox <gi...@apache.org>.
Indhumathi27 commented on a change in pull request #4076:
URL: https://github.com/apache/carbondata/pull/4076#discussion_r561714909
##########
File path: core/src/main/java/org/apache/carbondata/core/view/MVProvider.java
##########
@@ -569,14 +568,31 @@ private synchronized void touchMDTFile() throws IOException {
FileFactory.createDirectoryAndSetPermission(this.systemDirectory,
new FsPermission(FsAction.ALL, FsAction.ALL, FsAction.ALL));
}
- CarbonFile schemaIndexFile = FileFactory.getCarbonFile(this.schemaIndexFilePath);
- if (schemaIndexFile.exists()) {
- schemaIndexFile.delete();
+ // two or more JVM process can access this method to update last modified time at same
+ // time causing exception. So take a system level lock on system folder and update
+ // last modified time of schema index file
+ ICarbonLock systemDirLock = CarbonLockFactory
+ .getSystemLevelCarbonLockObj(this.systemDirectory,
+ LockUsage.MATERIALIZED_VIEW_STATUS_LOCK);
+ boolean locked = false;
+ try {
+ locked = systemDirLock.lockWithRetries();
+ if (locked) {
+ CarbonFile schemaIndexFile = FileFactory.getCarbonFile(this.schemaIndexFilePath);
+ if (schemaIndexFile.exists()) {
+ schemaIndexFile.delete();
+ }
+ schemaIndexFile.createNewFile(new FsPermission(FsAction.ALL, FsAction.ALL, FsAction.ALL));
+ this.lastModifiedTime = schemaIndexFile.getLastModifiedTime();
+ } else {
+ LOG.warn("Unable to get Lock to refresh schema index last modified time");
Review comment:
Not needed, As next operation will compare last modified time and update it
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [carbondata] Indhumathi27 commented on pull request #4076: [CARBONDATA-4107] Added related MV tables Map to fact table and added lock while touchMDTFile
Posted by GitBox <gi...@apache.org>.
Indhumathi27 commented on pull request #4076:
URL: https://github.com/apache/carbondata/pull/4076#issuecomment-766746411
retest this please
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [carbondata] CarbonDataQA2 commented on pull request #4076: [CARBONDATA-4107] Added mvExists property for MV fact table and added lock while touchMDTFile
Posted by GitBox <gi...@apache.org>.
CarbonDataQA2 commented on pull request #4076:
URL: https://github.com/apache/carbondata/pull/4076#issuecomment-764441692
Build Failed with Spark 2.4.5, Please check CI http://121.244.95.60:12444/job/ApacheCarbon_PR_Builder_2.4.5/3566/
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [carbondata] CarbonDataQA2 commented on pull request #4076: [CARBONDATA-4107] Added mvExists property for MV fact table and added lock while touchMDTFile
Posted by GitBox <gi...@apache.org>.
CarbonDataQA2 commented on pull request #4076:
URL: https://github.com/apache/carbondata/pull/4076#issuecomment-762724012
Build Failed with Spark 2.4.5, Please check CI http://121.244.95.60:12444/job/ApacheCarbon_PR_Builder_2.4.5/3555/
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [carbondata] Indhumathi27 commented on pull request #4076: [CARBONDATA-4107] Added mvExists property for MV fact table and added lock while touchMDTFile
Posted by GitBox <gi...@apache.org>.
Indhumathi27 commented on pull request #4076:
URL: https://github.com/apache/carbondata/pull/4076#issuecomment-764288658
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [carbondata] Indhumathi27 commented on pull request #4076: [CARBONDATA-4107] Added mvExists property for MV fact table and added lock while touchMDTFile
Posted by GitBox <gi...@apache.org>.
Indhumathi27 commented on pull request #4076:
URL: https://github.com/apache/carbondata/pull/4076#issuecomment-762702037
retest this please
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [carbondata] CarbonDataQA2 commented on pull request #4076: [CARBONDATA-4107] Added mvExists property for MV fact table and added lock while touchMDTFile
Posted by GitBox <gi...@apache.org>.
CarbonDataQA2 commented on pull request #4076:
URL: https://github.com/apache/carbondata/pull/4076#issuecomment-764411437
Build Failed with Spark 2.4.5, Please check CI http://121.244.95.60:12444/job/ApacheCarbon_PR_Builder_2.4.5/3565/
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [carbondata] CarbonDataQA2 commented on pull request #4076: [CARBONDATA-4107] Added related MV tables Map to fact table and added lock while touchMDTFile
Posted by GitBox <gi...@apache.org>.
CarbonDataQA2 commented on pull request #4076:
URL: https://github.com/apache/carbondata/pull/4076#issuecomment-771646152
Build Success with Spark 2.3.4, Please check CI http://121.244.95.60:12444/job/ApacheCarbonPRBuilder2.3/5412/
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [carbondata] CarbonDataQA2 commented on pull request #4076: [CARBONDATA-4107] Added mvExists property for MV fact table and added lock while touchMDTFile
Posted by GitBox <gi...@apache.org>.
CarbonDataQA2 commented on pull request #4076:
URL: https://github.com/apache/carbondata/pull/4076#issuecomment-764355741
Build Failed with Spark 2.4.5, Please check CI http://121.244.95.60:12444/job/ApacheCarbon_PR_Builder_2.4.5/3563/
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [carbondata] CarbonDataQA2 commented on pull request #4076: [WIP] Added mvExists property for MV fact table and added lock while touchMDTFile
Posted by GitBox <gi...@apache.org>.
CarbonDataQA2 commented on pull request #4076:
URL: https://github.com/apache/carbondata/pull/4076#issuecomment-758578862
Build Success with Spark 2.4.5, Please check CI http://121.244.95.60:12444/job/ApacheCarbon_PR_Builder_2.4.5/3543/
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [carbondata] CarbonDataQA2 commented on pull request #4076: [CARBONDATA-4107] Added mvExists property for MV fact table and added lock while touchMDTFile
Posted by GitBox <gi...@apache.org>.
CarbonDataQA2 commented on pull request #4076:
URL: https://github.com/apache/carbondata/pull/4076#issuecomment-759454088
Build Success with Spark 2.3.4, Please check CI http://121.244.95.60:12444/job/ApacheCarbonPRBuilder2.3/5308/
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [carbondata] CarbonDataQA2 commented on pull request #4076: [CARBONDATA-4107] Added related MV tables Map to fact table and added lock while touchMDTFile
Posted by GitBox <gi...@apache.org>.
CarbonDataQA2 commented on pull request #4076:
URL: https://github.com/apache/carbondata/pull/4076#issuecomment-766796210
Build Success with Spark 2.3.4, Please check CI http://121.244.95.60:12444/job/ApacheCarbonPRBuilder2.3/5353/
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [carbondata] akashrn5 commented on a change in pull request #4076: [CARBONDATA-4107] Added mvExists property for MV fact table and added lock while touchMDTFile
Posted by GitBox <gi...@apache.org>.
akashrn5 commented on a change in pull request #4076:
URL: https://github.com/apache/carbondata/pull/4076#discussion_r561671488
##########
File path: core/src/main/java/org/apache/carbondata/core/view/MVProvider.java
##########
@@ -569,14 +568,31 @@ private synchronized void touchMDTFile() throws IOException {
FileFactory.createDirectoryAndSetPermission(this.systemDirectory,
new FsPermission(FsAction.ALL, FsAction.ALL, FsAction.ALL));
}
- CarbonFile schemaIndexFile = FileFactory.getCarbonFile(this.schemaIndexFilePath);
- if (schemaIndexFile.exists()) {
- schemaIndexFile.delete();
+ // two or more JVM process can access this method to update last modified time at same
+ // time causing exception. So take a system level lock on system folder and update
+ // last modified time of schema index file
+ ICarbonLock systemDirLock = CarbonLockFactory
+ .getSystemLevelCarbonLockObj(this.systemDirectory,
+ LockUsage.MATERIALIZED_VIEW_STATUS_LOCK);
+ boolean locked = false;
+ try {
+ locked = systemDirLock.lockWithRetries();
+ if (locked) {
+ CarbonFile schemaIndexFile = FileFactory.getCarbonFile(this.schemaIndexFilePath);
+ if (schemaIndexFile.exists()) {
+ schemaIndexFile.delete();
+ }
+ schemaIndexFile.createNewFile(new FsPermission(FsAction.ALL, FsAction.ALL, FsAction.ALL));
+ this.lastModifiedTime = schemaIndexFile.getLastModifiedTime();
+ } else {
+ LOG.warn("Unable to get Lock to refresh schema index last modified time");
Review comment:
if lock acquire fails, just a warn, dont we need to fail? as it may lead to some other problem?
##########
File path: core/src/main/java/org/apache/carbondata/core/view/MVManager.java
##########
@@ -132,6 +108,31 @@ public boolean isMVInSyncWithParentTables(MVSchema mvSchema) throws IOException
return schemas;
}
+ /**
+ * It gives all mv schemas from given databases in the store
+ */
+ public List<MVSchema> getSchemas(Map<String, List<String>> mvTablesMap) throws IOException {
+ List<MVSchema> schemas = new ArrayList<>();
+ for (Map.Entry<String, List<String>> databaseEntry : mvTablesMap.entrySet()) {
+ String database = databaseEntry.getKey();
+ List<String> mvTables = databaseEntry.getValue();
+ for (String mvTable : mvTables) {
+ try {
+ schemas.add(this.getSchema(database, mvTable));
+ } catch (IOException ex) {
+ throw ex;
Review comment:
please add a error log here, before throwing exception
##########
File path: integration/spark/src/main/scala/org/apache/spark/sql/execution/command/view/CarbonDropMVCommand.scala
##########
@@ -90,6 +95,37 @@ case class CarbonDropMVCommand(
}
}
+ // Update the mvExists and related databases property to mv fact tables
+ schema.getRelatedTables.asScala.foreach { table =>
+ val dbName = table.getDatabaseName
+ val tableName = table.getTableName
+ try {
+ val carbonTable =
+ CarbonEnv.getCarbonTable(Some(dbName), tableName)(session)
+ val relatedMVTablesMap = carbonTable.getMVTablesMap
+ // check if database has materialized views or not
+ val anotherMVExistsInDb = viewManager.getSchemas(databaseName).asScala.exists {
+ mvSchema =>
+ mvSchema.getRelatedTables.asScala.exists(_.getTableName.equalsIgnoreCase(tableName))
+ }
+ if (!anotherMVExistsInDb) {
+ // If database dont have any MV, then remove the database from related tables
+ // property and update table property of fact table
+ relatedMVTablesMap.get(databaseName).remove(name)
+ if (relatedMVTablesMap.get(databaseName).isEmpty) {
+ relatedMVTablesMap.remove(databaseName)
+ }
+ CarbonIndexUtil.addOrModifyTableProperty(carbonTable,
+ Map("relatedMVTablesMap".toLowerCase -> new Gson().toJson(relatedMVTablesMap)),
+ needLock = false)(session)
+ CarbonHiveIndexMetadataUtil.refreshTable(dbName, tableName, session)
+ }
+ } catch {
+ case _: Exception =>
+ // ignore as table is a non-carbon table
Review comment:
this comment is wrong? this flow will be for carbon tables also right?
##########
File path: integration/spark/src/main/scala/org/apache/spark/sql/execution/command/view/CarbonCreateMVCommand.scala
##########
@@ -122,6 +130,30 @@ case class CarbonCreateMVCommand(
this.viewSchema = schema
+ // Update the mvExists and related databases property to mv fact tables
+ relatedTableList.asScala.foreach { parentTable =>
+ val dbName = parentTable.getDatabaseName
+ val tableName = parentTable.getTableName
+ try {
+ val carbonTable = CarbonEnv.getCarbonTable(Some(dbName), tableName)(session)
+ val relatedMVTablesMap = carbonTable.getMVTablesMap
+ if (!relatedMVTablesMap.containsKey(databaseName)) {
+ val mvTables = new util.ArrayList[String]()
+ mvTables.add(name)
+ relatedMVTablesMap.put(databaseName, mvTables)
+ } else if (!relatedMVTablesMap.get(databaseName).contains(name)) {
+ relatedMVTablesMap.get(databaseName).add(name)
+ }
+ CarbonIndexUtil.addOrModifyTableProperty(carbonTable,
+ Map("relatedMVTablesMap".toLowerCase -> new Gson().toJson(relatedMVTablesMap)),
Review comment:
`relatedMVTablesMap` used in multiple places, please add to a constant
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [carbondata] asfgit closed pull request #4076: [CARBONDATA-4107] Added related MV tables Map to fact table and added lock while touchMDTFile
Posted by GitBox <gi...@apache.org>.
asfgit closed pull request #4076:
URL: https://github.com/apache/carbondata/pull/4076
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [carbondata] CarbonDataQA2 commented on pull request #4076: [CARBONDATA-4107] Added related MV tables Map to fact table and added lock while touchMDTFile
Posted by GitBox <gi...@apache.org>.
CarbonDataQA2 commented on pull request #4076:
URL: https://github.com/apache/carbondata/pull/4076#issuecomment-764626476
Build Failed with Spark 2.4.5, Please check CI http://121.244.95.60:12444/job/ApacheCarbon_PR_Builder_2.4.5/3571/
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [carbondata] CarbonDataQA2 commented on pull request #4076: [CARBONDATA-4107] Added related MV tables Map to fact table and added lock while touchMDTFile
Posted by GitBox <gi...@apache.org>.
CarbonDataQA2 commented on pull request #4076:
URL: https://github.com/apache/carbondata/pull/4076#issuecomment-772475193
Build Success with Spark 2.3.4, Please check CI http://121.244.95.60:12444/job/ApacheCarbonPRBuilder2.3/5418/
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [carbondata] CarbonDataQA2 commented on pull request #4076: [CARBONDATA-4107] Added mvExists property for MV fact table and added lock while touchMDTFile
Posted by GitBox <gi...@apache.org>.
CarbonDataQA2 commented on pull request #4076:
URL: https://github.com/apache/carbondata/pull/4076#issuecomment-762672382
Build Failed with Spark 2.4.5, Please check CI http://121.244.95.60:12444/job/ApacheCarbon_PR_Builder_2.4.5/3554/
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [carbondata] akashrn5 commented on a change in pull request #4076: [CARBONDATA-4107] Added related MV tables Map to fact table and added lock while touchMDTFile
Posted by GitBox <gi...@apache.org>.
akashrn5 commented on a change in pull request #4076:
URL: https://github.com/apache/carbondata/pull/4076#discussion_r568501151
##########
File path: docs/mv-guide.md
##########
@@ -241,6 +242,10 @@ The current information includes:
| Refresh Mode | FULL / INCREMENTAL refresh to MV |
| Refresh Trigger Mode | ON_COMMIT / ON_MANUAL refresh to MV provided by user |
| Properties | Table properties of the materialized view |
+
+**NOTE**: For materialized views created
+before [CARBONDATA-4107](https://issues.apache.org/jira/browse/CARBONDATA-4107) issue fix, run
+refresh mv command to add mv name to fact table property and to enable it.
Review comment:
also please add what happens if user doesn't use refresh
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [carbondata] CarbonDataQA2 commented on pull request #4076: [CARBONDATA-4107] Added related MV tables Map to fact table and added lock while touchMDTFile
Posted by GitBox <gi...@apache.org>.
CarbonDataQA2 commented on pull request #4076:
URL: https://github.com/apache/carbondata/pull/4076#issuecomment-773157919
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [carbondata] CarbonDataQA2 commented on pull request #4076: [CARBONDATA-4107] Added mvExists property for MV fact table and added lock while touchMDTFile
Posted by GitBox <gi...@apache.org>.
CarbonDataQA2 commented on pull request #4076:
URL: https://github.com/apache/carbondata/pull/4076#issuecomment-764410335
Build Failed with Spark 2.3.4, Please check CI http://121.244.95.60:12444/job/ApacheCarbonPRBuilder2.3/5325/
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [carbondata] CarbonDataQA2 commented on pull request #4076: [CARBONDATA-4107] Added related MV tables Map to fact table and added lock while touchMDTFile
Posted by GitBox <gi...@apache.org>.
CarbonDataQA2 commented on pull request #4076:
URL: https://github.com/apache/carbondata/pull/4076#issuecomment-766567423
Build Failed with Spark 2.3.4, Please check CI http://121.244.95.60:12545/job/ApacheCarbonPRBuilder2.3/5049/
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [carbondata] akashrn5 commented on pull request #4076: [CARBONDATA-4107] Added related MV tables Map to fact table and added lock while touchMDTFile
Posted by GitBox <gi...@apache.org>.
akashrn5 commented on pull request #4076:
URL: https://github.com/apache/carbondata/pull/4076#issuecomment-768803929
LGTM
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [carbondata] Indhumathi27 commented on pull request #4076: [CARBONDATA-4107] Added related MV tables Map to fact table and added lock while touchMDTFile
Posted by GitBox <gi...@apache.org>.
Indhumathi27 commented on pull request #4076:
URL: https://github.com/apache/carbondata/pull/4076#issuecomment-768812884
@QiangCai Please review
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [carbondata] Indhumathi27 commented on pull request #4076: [CARBONDATA-4107] Added mvExists property for MV fact table and added lock while touchMDTFile
Posted by GitBox <gi...@apache.org>.
Indhumathi27 commented on pull request #4076:
URL: https://github.com/apache/carbondata/pull/4076#issuecomment-764447779
retest this please
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [carbondata] CarbonDataQA2 commented on pull request #4076: [WIP] Added mvExists property in MV fact table and lock while touchMDTFile
Posted by GitBox <gi...@apache.org>.
CarbonDataQA2 commented on pull request #4076:
URL: https://github.com/apache/carbondata/pull/4076#issuecomment-758502714
Build Failed with Spark 2.3.4, Please check CI http://121.244.95.60:12444/job/ApacheCarbonPRBuilder2.3/5301/
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [carbondata] akashrn5 commented on a change in pull request #4076: [CARBONDATA-4107] Added related MV tables Map to fact table and added lock while touchMDTFile
Posted by GitBox <gi...@apache.org>.
akashrn5 commented on a change in pull request #4076:
URL: https://github.com/apache/carbondata/pull/4076#discussion_r569996744
##########
File path: docs/mv-guide.md
##########
@@ -241,6 +242,11 @@ The current information includes:
| Refresh Mode | FULL / INCREMENTAL refresh to MV |
| Refresh Trigger Mode | ON_COMMIT / ON_MANUAL refresh to MV provided by user |
| Properties | Table properties of the materialized view |
+
+**NOTE**: For materialized views created
+before [CARBONDATA-4107](https://issues.apache.org/jira/browse/CARBONDATA-4107) issue fix, run
+refresh mv command to add mv name to fact table property and to enable it. If refresh command
Review comment:
```suggestion
refresh mv command to add mv name to fact table's table properties and to enable it. If refresh command
```
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [carbondata] CarbonDataQA2 commented on pull request #4076: [CARBONDATA-4107] Added related MV tables Map to fact table and added lock while touchMDTFile
Posted by GitBox <gi...@apache.org>.
CarbonDataQA2 commented on pull request #4076:
URL: https://github.com/apache/carbondata/pull/4076#issuecomment-765431275
Build Success with Spark 2.3.4, Please check CI http://121.244.95.60:12444/job/ApacheCarbonPRBuilder2.3/5338/
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [carbondata] CarbonDataQA2 commented on pull request #4076: [CARBONDATA-4107] Added related MV tables Map to fact table and added lock while touchMDTFile
Posted by GitBox <gi...@apache.org>.
CarbonDataQA2 commented on pull request #4076:
URL: https://github.com/apache/carbondata/pull/4076#issuecomment-768213920
Build Success with Spark 2.3.4, Please check CI http://121.244.95.60:12444/job/ApacheCarbonPRBuilder2.3/5361/
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [carbondata] CarbonDataQA2 commented on pull request #4076: [CARBONDATA-4107] Added related MV tables Map to fact table and added lock while touchMDTFile
Posted by GitBox <gi...@apache.org>.
CarbonDataQA2 commented on pull request #4076:
URL: https://github.com/apache/carbondata/pull/4076#issuecomment-771646152
Build Success with Spark 2.3.4, Please check CI http://121.244.95.60:12444/job/ApacheCarbonPRBuilder2.3/5412/
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [carbondata] akashrn5 commented on a change in pull request #4076: [CARBONDATA-4107] Added mvExists property for MV fact table and added lock while touchMDTFile
Posted by GitBox <gi...@apache.org>.
akashrn5 commented on a change in pull request #4076:
URL: https://github.com/apache/carbondata/pull/4076#discussion_r561671488
##########
File path: core/src/main/java/org/apache/carbondata/core/view/MVProvider.java
##########
@@ -569,14 +568,31 @@ private synchronized void touchMDTFile() throws IOException {
FileFactory.createDirectoryAndSetPermission(this.systemDirectory,
new FsPermission(FsAction.ALL, FsAction.ALL, FsAction.ALL));
}
- CarbonFile schemaIndexFile = FileFactory.getCarbonFile(this.schemaIndexFilePath);
- if (schemaIndexFile.exists()) {
- schemaIndexFile.delete();
+ // two or more JVM process can access this method to update last modified time at same
+ // time causing exception. So take a system level lock on system folder and update
+ // last modified time of schema index file
+ ICarbonLock systemDirLock = CarbonLockFactory
+ .getSystemLevelCarbonLockObj(this.systemDirectory,
+ LockUsage.MATERIALIZED_VIEW_STATUS_LOCK);
+ boolean locked = false;
+ try {
+ locked = systemDirLock.lockWithRetries();
+ if (locked) {
+ CarbonFile schemaIndexFile = FileFactory.getCarbonFile(this.schemaIndexFilePath);
+ if (schemaIndexFile.exists()) {
+ schemaIndexFile.delete();
+ }
+ schemaIndexFile.createNewFile(new FsPermission(FsAction.ALL, FsAction.ALL, FsAction.ALL));
+ this.lastModifiedTime = schemaIndexFile.getLastModifiedTime();
+ } else {
+ LOG.warn("Unable to get Lock to refresh schema index last modified time");
Review comment:
if lock acquire fails, just a warn, dont we need to fail? as it may lead to some other problem?
##########
File path: core/src/main/java/org/apache/carbondata/core/view/MVManager.java
##########
@@ -132,6 +108,31 @@ public boolean isMVInSyncWithParentTables(MVSchema mvSchema) throws IOException
return schemas;
}
+ /**
+ * It gives all mv schemas from given databases in the store
+ */
+ public List<MVSchema> getSchemas(Map<String, List<String>> mvTablesMap) throws IOException {
+ List<MVSchema> schemas = new ArrayList<>();
+ for (Map.Entry<String, List<String>> databaseEntry : mvTablesMap.entrySet()) {
+ String database = databaseEntry.getKey();
+ List<String> mvTables = databaseEntry.getValue();
+ for (String mvTable : mvTables) {
+ try {
+ schemas.add(this.getSchema(database, mvTable));
+ } catch (IOException ex) {
+ throw ex;
Review comment:
please add a error log here, before throwing exception
##########
File path: integration/spark/src/main/scala/org/apache/spark/sql/execution/command/view/CarbonDropMVCommand.scala
##########
@@ -90,6 +95,37 @@ case class CarbonDropMVCommand(
}
}
+ // Update the mvExists and related databases property to mv fact tables
+ schema.getRelatedTables.asScala.foreach { table =>
+ val dbName = table.getDatabaseName
+ val tableName = table.getTableName
+ try {
+ val carbonTable =
+ CarbonEnv.getCarbonTable(Some(dbName), tableName)(session)
+ val relatedMVTablesMap = carbonTable.getMVTablesMap
+ // check if database has materialized views or not
+ val anotherMVExistsInDb = viewManager.getSchemas(databaseName).asScala.exists {
+ mvSchema =>
+ mvSchema.getRelatedTables.asScala.exists(_.getTableName.equalsIgnoreCase(tableName))
+ }
+ if (!anotherMVExistsInDb) {
+ // If database dont have any MV, then remove the database from related tables
+ // property and update table property of fact table
+ relatedMVTablesMap.get(databaseName).remove(name)
+ if (relatedMVTablesMap.get(databaseName).isEmpty) {
+ relatedMVTablesMap.remove(databaseName)
+ }
+ CarbonIndexUtil.addOrModifyTableProperty(carbonTable,
+ Map("relatedMVTablesMap".toLowerCase -> new Gson().toJson(relatedMVTablesMap)),
+ needLock = false)(session)
+ CarbonHiveIndexMetadataUtil.refreshTable(dbName, tableName, session)
+ }
+ } catch {
+ case _: Exception =>
+ // ignore as table is a non-carbon table
Review comment:
this comment is wrong? this flow will be for carbon tables also right?
##########
File path: integration/spark/src/main/scala/org/apache/spark/sql/execution/command/view/CarbonCreateMVCommand.scala
##########
@@ -122,6 +130,30 @@ case class CarbonCreateMVCommand(
this.viewSchema = schema
+ // Update the mvExists and related databases property to mv fact tables
+ relatedTableList.asScala.foreach { parentTable =>
+ val dbName = parentTable.getDatabaseName
+ val tableName = parentTable.getTableName
+ try {
+ val carbonTable = CarbonEnv.getCarbonTable(Some(dbName), tableName)(session)
+ val relatedMVTablesMap = carbonTable.getMVTablesMap
+ if (!relatedMVTablesMap.containsKey(databaseName)) {
+ val mvTables = new util.ArrayList[String]()
+ mvTables.add(name)
+ relatedMVTablesMap.put(databaseName, mvTables)
+ } else if (!relatedMVTablesMap.get(databaseName).contains(name)) {
+ relatedMVTablesMap.get(databaseName).add(name)
+ }
+ CarbonIndexUtil.addOrModifyTableProperty(carbonTable,
+ Map("relatedMVTablesMap".toLowerCase -> new Gson().toJson(relatedMVTablesMap)),
Review comment:
`relatedMVTablesMap` used in multiple places, please add to a constant
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [carbondata] CarbonDataQA2 commented on pull request #4076: [CARBONDATA-4107] Added mvExists property for MV fact table and added lock while touchMDTFile
Posted by GitBox <gi...@apache.org>.
CarbonDataQA2 commented on pull request #4076:
URL: https://github.com/apache/carbondata/pull/4076#issuecomment-764440517
Build Failed with Spark 2.3.4, Please check CI http://121.244.95.60:12444/job/ApacheCarbonPRBuilder2.3/5326/
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [carbondata] Indhumathi27 commented on a change in pull request #4076: [CARBONDATA-4107] Added mvExists property for MV fact table and added lock while touchMDTFile
Posted by GitBox <gi...@apache.org>.
Indhumathi27 commented on a change in pull request #4076:
URL: https://github.com/apache/carbondata/pull/4076#discussion_r561693339
##########
File path: integration/spark/src/main/scala/org/apache/spark/sql/execution/command/view/CarbonDropMVCommand.scala
##########
@@ -90,6 +95,37 @@ case class CarbonDropMVCommand(
}
}
+ // Update the mvExists and related databases property to mv fact tables
+ schema.getRelatedTables.asScala.foreach { table =>
+ val dbName = table.getDatabaseName
+ val tableName = table.getTableName
+ try {
+ val carbonTable =
+ CarbonEnv.getCarbonTable(Some(dbName), tableName)(session)
+ val relatedMVTablesMap = carbonTable.getMVTablesMap
+ // check if database has materialized views or not
+ val anotherMVExistsInDb = viewManager.getSchemas(databaseName).asScala.exists {
+ mvSchema =>
+ mvSchema.getRelatedTables.asScala.exists(_.getTableName.equalsIgnoreCase(tableName))
+ }
+ if (!anotherMVExistsInDb) {
+ // If database dont have any MV, then remove the database from related tables
+ // property and update table property of fact table
+ relatedMVTablesMap.get(databaseName).remove(name)
+ if (relatedMVTablesMap.get(databaseName).isEmpty) {
+ relatedMVTablesMap.remove(databaseName)
+ }
+ CarbonIndexUtil.addOrModifyTableProperty(carbonTable,
+ Map("relatedMVTablesMap".toLowerCase -> new Gson().toJson(relatedMVTablesMap)),
+ needLock = false)(session)
+ CarbonHiveIndexMetadataUtil.refreshTable(dbName, tableName, session)
+ }
+ } catch {
+ case _: Exception =>
+ // ignore as table is a non-carbon table
Review comment:
This Exception is added, because for parquet and orc, carbon table will not be present.
##########
File path: core/src/main/java/org/apache/carbondata/core/view/MVProvider.java
##########
@@ -569,14 +568,31 @@ private synchronized void touchMDTFile() throws IOException {
FileFactory.createDirectoryAndSetPermission(this.systemDirectory,
new FsPermission(FsAction.ALL, FsAction.ALL, FsAction.ALL));
}
- CarbonFile schemaIndexFile = FileFactory.getCarbonFile(this.schemaIndexFilePath);
- if (schemaIndexFile.exists()) {
- schemaIndexFile.delete();
+ // two or more JVM process can access this method to update last modified time at same
+ // time causing exception. So take a system level lock on system folder and update
+ // last modified time of schema index file
+ ICarbonLock systemDirLock = CarbonLockFactory
+ .getSystemLevelCarbonLockObj(this.systemDirectory,
+ LockUsage.MATERIALIZED_VIEW_STATUS_LOCK);
+ boolean locked = false;
+ try {
+ locked = systemDirLock.lockWithRetries();
+ if (locked) {
+ CarbonFile schemaIndexFile = FileFactory.getCarbonFile(this.schemaIndexFilePath);
+ if (schemaIndexFile.exists()) {
+ schemaIndexFile.delete();
+ }
+ schemaIndexFile.createNewFile(new FsPermission(FsAction.ALL, FsAction.ALL, FsAction.ALL));
+ this.lastModifiedTime = schemaIndexFile.getLastModifiedTime();
+ } else {
+ LOG.warn("Unable to get Lock to refresh schema index last modified time");
Review comment:
Not needed, As next operation will compare last modified time and update it
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [carbondata] CarbonDataQA2 commented on pull request #4076: [CARBONDATA-4107] Added related MV tables Map to fact table and added lock while touchMDTFile
Posted by GitBox <gi...@apache.org>.
CarbonDataQA2 commented on pull request #4076:
URL: https://github.com/apache/carbondata/pull/4076#issuecomment-772478541
Build Success with Spark 2.4.5, Please check CI http://121.244.95.60:12444/job/ApacheCarbon_PR_Builder_2.4.5/3657/
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [carbondata] CarbonDataQA2 commented on pull request #4076: [CARBONDATA-4107] Added mvExists property for MV fact table and added lock while touchMDTFile
Posted by GitBox <gi...@apache.org>.
CarbonDataQA2 commented on pull request #4076:
URL: https://github.com/apache/carbondata/pull/4076#issuecomment-764464803
Build Failed with Spark 2.4.5, Please check CI http://121.244.95.60:12444/job/ApacheCarbon_PR_Builder_2.4.5/3568/
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [carbondata] akashrn5 commented on pull request #4076: [CARBONDATA-4107] Added related MV tables Map to fact table and added lock while touchMDTFile
Posted by GitBox <gi...@apache.org>.
akashrn5 commented on pull request #4076:
URL: https://github.com/apache/carbondata/pull/4076#issuecomment-773160807
LGTM
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [carbondata] CarbonDataQA2 commented on pull request #4076: [CARBONDATA-4107] Added related MV tables Map to fact table and added lock while touchMDTFile
Posted by GitBox <gi...@apache.org>.
CarbonDataQA2 commented on pull request #4076:
URL: https://github.com/apache/carbondata/pull/4076#issuecomment-773158309
Build Success with Spark 2.4.5, Please check CI http://121.244.95.60:12444/job/ApacheCarbon_PR_Builder_2.4.5/3669/
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [carbondata] CarbonDataQA2 commented on pull request #4076: [CARBONDATA-4107] Added related MV tables Map to fact table and added lock while touchMDTFile
Posted by GitBox <gi...@apache.org>.
CarbonDataQA2 commented on pull request #4076:
URL: https://github.com/apache/carbondata/pull/4076#issuecomment-764825371
Build Failed with Spark 2.4.5, Please check CI http://121.244.95.60:12444/job/ApacheCarbon_PR_Builder_2.4.5/3572/
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [carbondata] Indhumathi27 commented on pull request #4076: [CARBONDATA-4107] Added mvExists property for MV fact table and added lock while touchMDTFile
Posted by GitBox <gi...@apache.org>.
Indhumathi27 commented on pull request #4076:
URL: https://github.com/apache/carbondata/pull/4076#issuecomment-762229135
> @Indhumathi27 if the number of mv tables is less than 10 (configurable), maybe we can list all tables; if-else we can store database information only.
actually, here we are reading MV schemas. Mv schemas for a fact table can be present in any db (join scenarios).. so, we need to know the database also. So, we have to store Database_Name.MV_Name in fact table, which can increase fact table size. Still, you want to store mv tables in fact table?
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [carbondata] akashrn5 commented on a change in pull request #4076: [CARBONDATA-4107] Added related MV tables Map to fact table and added lock while touchMDTFile
Posted by GitBox <gi...@apache.org>.
akashrn5 commented on a change in pull request #4076:
URL: https://github.com/apache/carbondata/pull/4076#discussion_r565042150
##########
File path: core/src/main/java/org/apache/carbondata/core/constants/CarbonCommonConstants.java
##########
@@ -1222,6 +1222,11 @@ private CarbonCommonConstants() {
public static final String CARBON_ENABLE_MV_DEFAULT = "true";
+ /**
+ * Related mv table's map for a fact table
+ */
+ public static final String RELATED_MV_TABLES_MAP = "relatedMVTablesMap";
Review comment:
since you are always making lower case in usage, define in lowercase itself and avoid converting
##########
File path: integration/spark/src/main/scala/org/apache/spark/sql/execution/command/view/CarbonDropMVCommand.scala
##########
@@ -38,7 +38,8 @@ case class CarbonDropMVCommand(
databaseNameOption: Option[String],
name: String,
ifExistsSet: Boolean,
- forceDrop: Boolean = false)
+ forceDrop: Boolean = false,
+ lockAcquiredOnFactTable: String = null)
Review comment:
```suggestion
isLockAcquiredOnFactTable: String = null)
```
please change other places also
##########
File path: integration/spark/src/main/scala/org/apache/carbondata/view/MVHelper.scala
##########
@@ -372,6 +380,89 @@ object MVHelper {
updatedName
}
+ /**
+ * Add or modify the MV database and table name to fact table's related mv tables Map
+ */
+ def addOrModifyMVTablesMap(session: SparkSession,
Review comment:
please add the scenarios where this method is called and how handled
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [carbondata] QiangCai commented on pull request #4076: [CARBONDATA-4107] Added mvExists property for MV fact table and added lock while touchMDTFile
Posted by GitBox <gi...@apache.org>.
QiangCai commented on pull request #4076:
URL: https://github.com/apache/carbondata/pull/4076#issuecomment-762546050
yes, it can avoid to list all tables of some databases.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [carbondata] CarbonDataQA2 commented on pull request #4076: [CARBONDATA-4107] Added related MV tables Map to fact table and added lock while touchMDTFile
Posted by GitBox <gi...@apache.org>.
CarbonDataQA2 commented on pull request #4076:
URL: https://github.com/apache/carbondata/pull/4076#issuecomment-769713990
Build Success with Spark 2.4.5, Please check CI http://121.244.95.60:12444/job/ApacheCarbon_PR_Builder_2.4.5/3617/
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [carbondata] CarbonDataQA2 commented on pull request #4076: [CARBONDATA-4107] Added mvExists property for MV fact table and added lock while touchMDTFile
Posted by GitBox <gi...@apache.org>.
CarbonDataQA2 commented on pull request #4076:
URL: https://github.com/apache/carbondata/pull/4076#issuecomment-764355741
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [carbondata] CarbonDataQA2 commented on pull request #4076: [CARBONDATA-4107] Added mvExists property for MV fact table and added lock while touchMDTFile
Posted by GitBox <gi...@apache.org>.
CarbonDataQA2 commented on pull request #4076:
URL: https://github.com/apache/carbondata/pull/4076#issuecomment-762721960
Build Failed with Spark 2.3.4, Please check CI http://121.244.95.60:12444/job/ApacheCarbonPRBuilder2.3/5315/
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [carbondata] CarbonDataQA2 commented on pull request #4076: [CARBONDATA-4107] Added related MV tables Map to fact table and added lock while touchMDTFile
Posted by GitBox <gi...@apache.org>.
CarbonDataQA2 commented on pull request #4076:
URL: https://github.com/apache/carbondata/pull/4076#issuecomment-764822381
Build Failed with Spark 2.3.4, Please check CI http://121.244.95.60:12444/job/ApacheCarbonPRBuilder2.3/5332/
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [carbondata] CarbonDataQA2 commented on pull request #4076: [CARBONDATA-4107] Added related MV tables Map to fact table and added lock while touchMDTFile
Posted by GitBox <gi...@apache.org>.
CarbonDataQA2 commented on pull request #4076:
URL: https://github.com/apache/carbondata/pull/4076#issuecomment-766553406
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [carbondata] Indhumathi27 edited a comment on pull request #4076: [CARBONDATA-4107] Added mvExists property for MV fact table and added lock while touchMDTFile
Posted by GitBox <gi...@apache.org>.
Indhumathi27 edited a comment on pull request #4076:
URL: https://github.com/apache/carbondata/pull/4076#issuecomment-762229135
> @Indhumathi27 if the number of mv tables is less than 10 (configurable), maybe we can list all tables; if-else we can store database information only.
actually, here we are reading MV schemas. Mv schemas for a fact table can be present in any db (join scenarios).. so, we need to know the database also. So, we have to store Database_Name.MV_Name in fact table, which can increase fact table size. Still, you want to store mv tables in fact table?
@QiangCai @akashrn5
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [carbondata] CarbonDataQA2 commented on pull request #4076: [CARBONDATA-4107] Added related MV tables Map to fact table and added lock while touchMDTFile
Posted by GitBox <gi...@apache.org>.
CarbonDataQA2 commented on pull request #4076:
URL: https://github.com/apache/carbondata/pull/4076#issuecomment-764623559
Build Failed with Spark 2.3.4, Please check CI http://121.244.95.60:12444/job/ApacheCarbonPRBuilder2.3/5331/
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [carbondata] CarbonDataQA2 commented on pull request #4076: [CARBONDATA-4107] Added related MV tables Map to fact table and added lock while touchMDTFile
Posted by GitBox <gi...@apache.org>.
CarbonDataQA2 commented on pull request #4076:
URL: https://github.com/apache/carbondata/pull/4076#issuecomment-765227702
Build Failed with Spark 2.4.5, Please check CI http://121.244.95.60:12444/job/ApacheCarbon_PR_Builder_2.4.5/3574/
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [carbondata] CarbonDataQA2 commented on pull request #4076: [CARBONDATA-4107] Added related MV tables Map to fact table and added lock while touchMDTFile
Posted by GitBox <gi...@apache.org>.
CarbonDataQA2 commented on pull request #4076:
URL: https://github.com/apache/carbondata/pull/4076#issuecomment-765348181
Build Success with Spark 2.4.5, Please check CI http://121.244.95.60:12444/job/ApacheCarbon_PR_Builder_2.4.5/3576/
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [carbondata] CarbonDataQA2 commented on pull request #4076: [CARBONDATA-4107] Added related MV tables Map to fact table and added lock while touchMDTFile
Posted by GitBox <gi...@apache.org>.
CarbonDataQA2 commented on pull request #4076:
URL: https://github.com/apache/carbondata/pull/4076#issuecomment-773157919
Build Success with Spark 2.3.4, Please check CI http://121.244.95.60:12444/job/ApacheCarbonPRBuilder2.3/5429/
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org