You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@carbondata.apache.org by ravipesala <gi...@git.apache.org> on 2018/04/16 13:00:58 UTC
[GitHub] carbondata pull request #2176: [CARBONDATA-2353] Added cache for datamap sch...
GitHub user ravipesala opened a pull request:
https://github.com/apache/carbondata/pull/2176
[CARBONDATA-2353] Added cache for datamap schema provider and added tests
Problem:
Currently, there is no cache for datamap schema provider, so every time it reads schema from disk.
Solution:
Add cache to the DiskBasedDMSchemaStorageProvider and refresh the cache depends on the modified time of datamap mdt file.
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
Tests added
- [ ] 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/ravipesala/incubator-carbondata schemasave
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/carbondata/pull/2176.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 #2176
----
commit d60e090b506ea0c91ea7144c421a6dbe6388daf0
Author: ravipesala <ra...@...>
Date: 2018-04-16T12:52:23Z
Added cache for datamap schema provider and added tests
----
---
[GitHub] carbondata issue #2176: [CARBONDATA-2353] Added cache for datamap schema pro...
Posted by CarbonDataQA <gi...@git.apache.org>.
Github user CarbonDataQA commented on the issue:
https://github.com/apache/carbondata/pull/2176
Build Success with Spark 2.2.1, Please check CI http://88.99.58.216:8080/job/ApacheCarbonPRBuilder/3899/
---
[GitHub] carbondata issue #2176: [CARBONDATA-2353] Added cache for datamap schema pro...
Posted by QiangCai <gi...@git.apache.org>.
Github user QiangCai commented on the issue:
https://github.com/apache/carbondata/pull/2176
LGTM
---
[GitHub] carbondata issue #2176: [CARBONDATA-2353] Added cache for datamap schema pro...
Posted by CarbonDataQA <gi...@git.apache.org>.
Github user CarbonDataQA commented on the issue:
https://github.com/apache/carbondata/pull/2176
Build Failed with Spark 2.1.0, Please check CI http://136.243.101.176:8080/job/ApacheCarbonPRBuilder1/5112/
---
[GitHub] carbondata pull request #2176: [CARBONDATA-2353] Added cache for datamap sch...
Posted by ravipesala <gi...@git.apache.org>.
Github user ravipesala commented on a diff in the pull request:
https://github.com/apache/carbondata/pull/2176#discussion_r182136198
--- Diff: core/src/main/java/org/apache/carbondata/core/metadata/schema/table/DiskBasedDMSchemaStorageProvider.java ---
@@ -135,9 +161,49 @@ public DiskBasedDMSchemaStorageProvider(String storePath) {
if (!FileFactory.isFileExist(schemaPath, FileFactory.getFileType(schemaPath))) {
throw new IOException("DataMap with name " + dataMapName + " does not exists in storage");
}
-
+ DataMapSchema dataMapSchemaToRemove = null;
+ for (DataMapSchema dataMapSchema : dataMapSchemas) {
+ if (dataMapSchema.getDataMapName().equalsIgnoreCase(dataMapName)) {
+ dataMapSchemaToRemove = dataMapSchema;
+ }
+ }
+ if (dataMapSchemaToRemove != null) {
+ dataMapSchemas.remove(dataMapSchemaToRemove);
+ }
if (!FileFactory.deleteFile(schemaPath, FileFactory.getFileType(schemaPath))) {
throw new IOException("DataMap with name " + dataMapName + " cannot be deleted");
+ } else {
+ touchMDTFile();
+ }
+ }
+
+ private void checkAndReloadDataMapSchemas() throws IOException {
+ if (FileFactory.isFileExist(mdtFilePath)) {
+ long lastModifiedTime = FileFactory.getCarbonFile(mdtFilePath).getLastModifiedTime();
+ if (this.lastModifiedTime != lastModifiedTime) {
+ dataMapSchemas = retrieveAllSchemasInternal();
+ this.lastModifiedTime = lastModifiedTime;
+ }
+ } else {
+ touchMDTFile();
+ retrieveAllSchemasInternal();
--- End diff --
ok
---
[GitHub] carbondata pull request #2176: [CARBONDATA-2353] Added cache for datamap sch...
Posted by QiangCai <gi...@git.apache.org>.
Github user QiangCai commented on a diff in the pull request:
https://github.com/apache/carbondata/pull/2176#discussion_r182000018
--- Diff: core/src/main/java/org/apache/carbondata/core/metadata/schema/table/DiskBasedDMSchemaStorageProvider.java ---
@@ -135,9 +161,49 @@ public DiskBasedDMSchemaStorageProvider(String storePath) {
if (!FileFactory.isFileExist(schemaPath, FileFactory.getFileType(schemaPath))) {
throw new IOException("DataMap with name " + dataMapName + " does not exists in storage");
}
-
+ DataMapSchema dataMapSchemaToRemove = null;
+ for (DataMapSchema dataMapSchema : dataMapSchemas) {
+ if (dataMapSchema.getDataMapName().equalsIgnoreCase(dataMapName)) {
+ dataMapSchemaToRemove = dataMapSchema;
+ }
+ }
+ if (dataMapSchemaToRemove != null) {
+ dataMapSchemas.remove(dataMapSchemaToRemove);
+ }
if (!FileFactory.deleteFile(schemaPath, FileFactory.getFileType(schemaPath))) {
throw new IOException("DataMap with name " + dataMapName + " cannot be deleted");
+ } else {
+ touchMDTFile();
+ }
+ }
+
+ private void checkAndReloadDataMapSchemas() throws IOException {
+ if (FileFactory.isFileExist(mdtFilePath)) {
+ long lastModifiedTime = FileFactory.getCarbonFile(mdtFilePath).getLastModifiedTime();
+ if (this.lastModifiedTime != lastModifiedTime) {
+ dataMapSchemas = retrieveAllSchemasInternal();
+ this.lastModifiedTime = lastModifiedTime;
+ }
+ } else {
+ touchMDTFile();
+ retrieveAllSchemasInternal();
--- End diff --
set result to dataMapSchemas
---
[GitHub] carbondata issue #2176: [CARBONDATA-2353] Added cache for datamap schema pro...
Posted by CarbonDataQA <gi...@git.apache.org>.
Github user CarbonDataQA commented on the issue:
https://github.com/apache/carbondata/pull/2176
Build Failed with Spark 2.2.1, Please check CI http://88.99.58.216:8080/job/ApacheCarbonPRBuilder/3905/
---
[GitHub] carbondata issue #2176: [CARBONDATA-2353] Added cache for datamap schema pro...
Posted by QiangCai <gi...@git.apache.org>.
Github user QiangCai commented on the issue:
https://github.com/apache/carbondata/pull/2176
please rebase master
---
[GitHub] carbondata pull request #2176: [CARBONDATA-2353] Added cache for datamap sch...
Posted by ravipesala <gi...@git.apache.org>.
Github user ravipesala commented on a diff in the pull request:
https://github.com/apache/carbondata/pull/2176#discussion_r182135807
--- Diff: core/src/main/java/org/apache/carbondata/core/datamap/DataMapStoreManager.java ---
@@ -111,47 +114,40 @@ private DataMapStoreManager() {
}
/**
- * It gives all datamap schemas.
+ * It gives all datamap schemas of a given table.
*
- * @return
*/
- public List<DataMapSchema> getAllDataMapSchemas(CarbonTable carbonTable) {
- // TODO cache all schemas and update only when datamap status file updates
- List<DataMapSchema> dataMapSchemas = getAllDataMapSchemas();
- List<DataMapSchema> dataMaps = new ArrayList<>();
- if (dataMapSchemas != null) {
- for (DataMapSchema dataMapSchema : dataMapSchemas) {
- RelationIdentifier identifier = dataMapSchema.getParentTables().get(0);
- if (dataMapSchema.isIndexDataMap() && identifier.getTableName()
- .equals(carbonTable.getTableName()) && identifier.getDatabaseName()
- .equals(carbonTable.getDatabaseName())) {
- dataMaps.add(dataMapSchema);
- }
- }
- }
- return dataMaps;
+ public List<DataMapSchema> getDataMapSchemasOfTable(CarbonTable carbonTable) throws IOException {
+ return provider.retrieveSchemas(carbonTable);
}
- public List<DataMapSchema> getAllDataMapSchemas() {
- DataMapSchemaStorageProvider provider = new DiskBasedDMSchemaStorageProvider(
- CarbonProperties.getInstance().getSystemFolderLocation());
- List<DataMapSchema> dataMapSchemas;
- try {
- dataMapSchemas = provider.retrieveAllSchemas();
- } catch (IOException e) {
- throw new RuntimeException(e);
- }
- return dataMapSchemas;
+ /**
+ * It gives all datamap schemas from store.
+ */
+ public List<DataMapSchema> getAllDataMapSchemas() throws IOException {
+ return provider.retrieveAllSchemas();
}
- public DataMapSchema getDataMapSchema(String dataMapName) throws NoSuchDataMapException {
- List<DataMapSchema> allDataMapSchemas = getAllDataMapSchemas();
- for (DataMapSchema dataMapSchema : allDataMapSchemas) {
- if (dataMapSchema.getDataMapName().equalsIgnoreCase(dataMapName)) {
- return dataMapSchema;
- }
- }
- throw new NoSuchDataMapException(dataMapName);
+
+ public DataMapSchema getDataMapSchema(String dataMapName)
+ throws NoSuchDataMapException, IOException {
+ return provider.retrieveSchema(dataMapName);
+ }
+
+ /**
+ * Saves the datamap schema to storage
+ * @param dataMapSchema
+ */
+ public void saveDataMapSchema(DataMapSchema dataMapSchema) throws IOException {
+ provider.saveSchema(dataMapSchema);
+ }
+
+ /**
+ * Saves the datamap schema to storage
--- End diff --
ok
---
[GitHub] carbondata issue #2176: [CARBONDATA-2353] Added cache for datamap schema pro...
Posted by QiangCai <gi...@git.apache.org>.
Github user QiangCai commented on the issue:
https://github.com/apache/carbondata/pull/2176
please fix CI failure
---
[GitHub] carbondata issue #2176: [CARBONDATA-2353] Added cache for datamap schema pro...
Posted by CarbonDataQA <gi...@git.apache.org>.
Github user CarbonDataQA commented on the issue:
https://github.com/apache/carbondata/pull/2176
Build Failed with Spark 2.2.1, Please check CI http://88.99.58.216:8080/job/ApacheCarbonPRBuilder/3894/
---
[GitHub] carbondata issue #2176: [CARBONDATA-2353] Added cache for datamap schema pro...
Posted by CarbonDataQA <gi...@git.apache.org>.
Github user CarbonDataQA commented on the issue:
https://github.com/apache/carbondata/pull/2176
Build Success with Spark 2.1.0, Please check CI http://136.243.101.176:8080/job/ApacheCarbonPRBuilder1/5129/
---
[GitHub] carbondata pull request #2176: [CARBONDATA-2353] Added cache for datamap sch...
Posted by QiangCai <gi...@git.apache.org>.
Github user QiangCai commented on a diff in the pull request:
https://github.com/apache/carbondata/pull/2176#discussion_r182000845
--- Diff: core/src/main/java/org/apache/carbondata/core/metadata/schema/table/DiskBasedDMSchemaStorageProvider.java ---
@@ -135,9 +161,49 @@ public DiskBasedDMSchemaStorageProvider(String storePath) {
if (!FileFactory.isFileExist(schemaPath, FileFactory.getFileType(schemaPath))) {
throw new IOException("DataMap with name " + dataMapName + " does not exists in storage");
}
-
+ DataMapSchema dataMapSchemaToRemove = null;
+ for (DataMapSchema dataMapSchema : dataMapSchemas) {
+ if (dataMapSchema.getDataMapName().equalsIgnoreCase(dataMapName)) {
+ dataMapSchemaToRemove = dataMapSchema;
+ }
+ }
+ if (dataMapSchemaToRemove != null) {
+ dataMapSchemas.remove(dataMapSchemaToRemove);
+ }
if (!FileFactory.deleteFile(schemaPath, FileFactory.getFileType(schemaPath))) {
throw new IOException("DataMap with name " + dataMapName + " cannot be deleted");
+ } else {
+ touchMDTFile();
+ }
+ }
+
+ private void checkAndReloadDataMapSchemas() throws IOException {
+ if (FileFactory.isFileExist(mdtFilePath)) {
+ long lastModifiedTime = FileFactory.getCarbonFile(mdtFilePath).getLastModifiedTime();
+ if (this.lastModifiedTime != lastModifiedTime) {
+ dataMapSchemas = retrieveAllSchemasInternal();
+ this.lastModifiedTime = lastModifiedTime;
+ }
+ } else {
+ touchMDTFile();
+ retrieveAllSchemasInternal();
+ }
+ }
+
+ private void touchMDTFile() throws IOException {
+ if (!FileFactory.isFileExist(storePath)) {
+ FileFactory.createDirectoryAndSetPermission(
+ storePath,
+ new FsPermission(FsAction.ALL, FsAction.ALL, FsAction.ALL));
+ }
+ if (!FileFactory.isFileExist(mdtFilePath)) {
+ FileFactory
+ .createNewFile(mdtFilePath,
--- End diff --
code style: move to last line
---
[GitHub] carbondata issue #2176: [CARBONDATA-2353] Added cache for datamap schema pro...
Posted by jackylk <gi...@git.apache.org>.
Github user jackylk commented on the issue:
https://github.com/apache/carbondata/pull/2176
LGTM
---
[GitHub] carbondata issue #2176: [CARBONDATA-2353] Added cache for datamap schema pro...
Posted by CarbonDataQA <gi...@git.apache.org>.
Github user CarbonDataQA commented on the issue:
https://github.com/apache/carbondata/pull/2176
Build Success with Spark 2.2.1, Please check CI http://88.99.58.216:8080/job/ApacheCarbonPRBuilder/3908/
---
[GitHub] carbondata issue #2176: [CARBONDATA-2353] Added cache for datamap schema pro...
Posted by ravipesala <gi...@git.apache.org>.
Github user ravipesala commented on the issue:
https://github.com/apache/carbondata/pull/2176
@QiangCai squashed commits, please merge
---
[GitHub] carbondata issue #2176: [CARBONDATA-2353] Added cache for datamap schema pro...
Posted by CarbonDataQA <gi...@git.apache.org>.
Github user CarbonDataQA commented on the issue:
https://github.com/apache/carbondata/pull/2176
Build Success with Spark 2.2.1, Please check CI http://88.99.58.216:8080/job/ApacheCarbonPRBuilder/3919/
---
[GitHub] carbondata pull request #2176: [CARBONDATA-2353] Added cache for datamap sch...
Posted by QiangCai <gi...@git.apache.org>.
Github user QiangCai commented on a diff in the pull request:
https://github.com/apache/carbondata/pull/2176#discussion_r181979047
--- Diff: core/src/main/java/org/apache/carbondata/core/datamap/DataMapStoreManager.java ---
@@ -111,47 +114,40 @@ private DataMapStoreManager() {
}
/**
- * It gives all datamap schemas.
+ * It gives all datamap schemas of a given table.
*
- * @return
*/
- public List<DataMapSchema> getAllDataMapSchemas(CarbonTable carbonTable) {
- // TODO cache all schemas and update only when datamap status file updates
- List<DataMapSchema> dataMapSchemas = getAllDataMapSchemas();
- List<DataMapSchema> dataMaps = new ArrayList<>();
- if (dataMapSchemas != null) {
- for (DataMapSchema dataMapSchema : dataMapSchemas) {
- RelationIdentifier identifier = dataMapSchema.getParentTables().get(0);
- if (dataMapSchema.isIndexDataMap() && identifier.getTableName()
- .equals(carbonTable.getTableName()) && identifier.getDatabaseName()
- .equals(carbonTable.getDatabaseName())) {
- dataMaps.add(dataMapSchema);
- }
- }
- }
- return dataMaps;
+ public List<DataMapSchema> getDataMapSchemasOfTable(CarbonTable carbonTable) throws IOException {
+ return provider.retrieveSchemas(carbonTable);
}
- public List<DataMapSchema> getAllDataMapSchemas() {
- DataMapSchemaStorageProvider provider = new DiskBasedDMSchemaStorageProvider(
- CarbonProperties.getInstance().getSystemFolderLocation());
- List<DataMapSchema> dataMapSchemas;
- try {
- dataMapSchemas = provider.retrieveAllSchemas();
- } catch (IOException e) {
- throw new RuntimeException(e);
- }
- return dataMapSchemas;
+ /**
+ * It gives all datamap schemas from store.
+ */
+ public List<DataMapSchema> getAllDataMapSchemas() throws IOException {
+ return provider.retrieveAllSchemas();
}
- public DataMapSchema getDataMapSchema(String dataMapName) throws NoSuchDataMapException {
- List<DataMapSchema> allDataMapSchemas = getAllDataMapSchemas();
- for (DataMapSchema dataMapSchema : allDataMapSchemas) {
- if (dataMapSchema.getDataMapName().equalsIgnoreCase(dataMapName)) {
- return dataMapSchema;
- }
- }
- throw new NoSuchDataMapException(dataMapName);
+
+ public DataMapSchema getDataMapSchema(String dataMapName)
+ throws NoSuchDataMapException, IOException {
+ return provider.retrieveSchema(dataMapName);
+ }
+
+ /**
+ * Saves the datamap schema to storage
+ * @param dataMapSchema
+ */
+ public void saveDataMapSchema(DataMapSchema dataMapSchema) throws IOException {
+ provider.saveSchema(dataMapSchema);
+ }
+
+ /**
+ * Saves the datamap schema to storage
--- End diff --
please correct comment
---
[GitHub] carbondata pull request #2176: [CARBONDATA-2353] Added cache for datamap sch...
Posted by asfgit <gi...@git.apache.org>.
Github user asfgit closed the pull request at:
https://github.com/apache/carbondata/pull/2176
---
[GitHub] carbondata issue #2176: [CARBONDATA-2353] Added cache for datamap schema pro...
Posted by QiangCai <gi...@git.apache.org>.
Github user QiangCai commented on the issue:
https://github.com/apache/carbondata/pull/2176
please rebase again, because there is merge conflict during rebase apache master
---
[GitHub] carbondata issue #2176: [CARBONDATA-2353] Added cache for datamap schema pro...
Posted by CarbonDataQA <gi...@git.apache.org>.
Github user CarbonDataQA commented on the issue:
https://github.com/apache/carbondata/pull/2176
Build Failed with Spark 2.2.1, Please check CI http://88.99.58.216:8080/job/ApacheCarbonPRBuilder/3852/
---
[GitHub] carbondata issue #2176: [CARBONDATA-2353] Added cache for datamap schema pro...
Posted by CarbonDataQA <gi...@git.apache.org>.
Github user CarbonDataQA commented on the issue:
https://github.com/apache/carbondata/pull/2176
Build Success with Spark 2.1.0, Please check CI http://136.243.101.176:8080/job/ApacheCarbonPRBuilder1/5140/
---
[GitHub] carbondata issue #2176: [CARBONDATA-2353] Added cache for datamap schema pro...
Posted by CarbonDataQA <gi...@git.apache.org>.
Github user CarbonDataQA commented on the issue:
https://github.com/apache/carbondata/pull/2176
Build Failed with Spark 2.1.0, Please check CI http://136.243.101.176:8080/job/ApacheCarbonPRBuilder1/5063/
---
[GitHub] carbondata issue #2176: [CARBONDATA-2353] Added cache for datamap schema pro...
Posted by jackylk <gi...@git.apache.org>.
Github user jackylk commented on the issue:
https://github.com/apache/carbondata/pull/2176
LGTM
---
[GitHub] carbondata issue #2176: [CARBONDATA-2353] Added cache for datamap schema pro...
Posted by CarbonDataQA <gi...@git.apache.org>.
Github user CarbonDataQA commented on the issue:
https://github.com/apache/carbondata/pull/2176
Build Success with Spark 2.1.0, Please check CI http://136.243.101.176:8080/job/ApacheCarbonPRBuilder1/5120/
---
[GitHub] carbondata issue #2176: [CARBONDATA-2353] Added cache for datamap schema pro...
Posted by CarbonDataQA <gi...@git.apache.org>.
Github user CarbonDataQA commented on the issue:
https://github.com/apache/carbondata/pull/2176
Build Failed with Spark 2.2.1, Please check CI http://88.99.58.216:8080/job/ApacheCarbonPRBuilder/3841/
---
[GitHub] carbondata issue #2176: [CARBONDATA-2353] Added cache for datamap schema pro...
Posted by CarbonDataQA <gi...@git.apache.org>.
Github user CarbonDataQA commented on the issue:
https://github.com/apache/carbondata/pull/2176
Build Failed with Spark 2.1.0, Please check CI http://136.243.101.176:8080/job/ApacheCarbonPRBuilder1/5115/
---
[GitHub] carbondata pull request #2176: [CARBONDATA-2353] Added cache for datamap sch...
Posted by ravipesala <gi...@git.apache.org>.
Github user ravipesala commented on a diff in the pull request:
https://github.com/apache/carbondata/pull/2176#discussion_r182136599
--- Diff: core/src/main/java/org/apache/carbondata/core/metadata/schema/table/DiskBasedDMSchemaStorageProvider.java ---
@@ -135,9 +161,49 @@ public DiskBasedDMSchemaStorageProvider(String storePath) {
if (!FileFactory.isFileExist(schemaPath, FileFactory.getFileType(schemaPath))) {
throw new IOException("DataMap with name " + dataMapName + " does not exists in storage");
}
-
+ DataMapSchema dataMapSchemaToRemove = null;
+ for (DataMapSchema dataMapSchema : dataMapSchemas) {
+ if (dataMapSchema.getDataMapName().equalsIgnoreCase(dataMapName)) {
+ dataMapSchemaToRemove = dataMapSchema;
+ }
+ }
+ if (dataMapSchemaToRemove != null) {
+ dataMapSchemas.remove(dataMapSchemaToRemove);
+ }
if (!FileFactory.deleteFile(schemaPath, FileFactory.getFileType(schemaPath))) {
throw new IOException("DataMap with name " + dataMapName + " cannot be deleted");
+ } else {
+ touchMDTFile();
+ }
+ }
+
+ private void checkAndReloadDataMapSchemas() throws IOException {
+ if (FileFactory.isFileExist(mdtFilePath)) {
+ long lastModifiedTime = FileFactory.getCarbonFile(mdtFilePath).getLastModifiedTime();
+ if (this.lastModifiedTime != lastModifiedTime) {
+ dataMapSchemas = retrieveAllSchemasInternal();
+ this.lastModifiedTime = lastModifiedTime;
+ }
+ } else {
+ touchMDTFile();
+ retrieveAllSchemasInternal();
+ }
+ }
+
+ private void touchMDTFile() throws IOException {
+ if (!FileFactory.isFileExist(storePath)) {
+ FileFactory.createDirectoryAndSetPermission(
+ storePath,
+ new FsPermission(FsAction.ALL, FsAction.ALL, FsAction.ALL));
+ }
+ if (!FileFactory.isFileExist(mdtFilePath)) {
+ FileFactory
+ .createNewFile(mdtFilePath,
--- End diff --
ok
---
[GitHub] carbondata issue #2176: [CARBONDATA-2353] Added cache for datamap schema pro...
Posted by CarbonDataQA <gi...@git.apache.org>.
Github user CarbonDataQA commented on the issue:
https://github.com/apache/carbondata/pull/2176
Build Failed with Spark 2.1.0, Please check CI http://136.243.101.176:8080/job/ApacheCarbonPRBuilder1/5074/
---
[GitHub] carbondata issue #2176: [CARBONDATA-2353] Added cache for datamap schema pro...
Posted by CarbonDataQA <gi...@git.apache.org>.
Github user CarbonDataQA commented on the issue:
https://github.com/apache/carbondata/pull/2176
Build Failed with Spark 2.2.1, Please check CI http://88.99.58.216:8080/job/ApacheCarbonPRBuilder/3891/
---