[GitHub] [carbondata] ravipesala commented on a change in pull request #3339: [CARBONDATA-3480] Fixed unnecessary refresh for table by removing modified mdt file

[GitBox <gi...@apache.org>]

ravipesala commented on a change in pull request #3339: [CARBONDATA-3480] Fixed unnecessary refresh for table by removing modified mdt file
URL: https://github.com/apache/carbondata/pull/3339#discussion_r309618536
 
 

 ##########
 File path: integration/spark2/src/main/scala/org/apache/spark/sql/hive/CarbonFileMetastore.scala
 ##########
 @@ -537,92 +482,38 @@ class CarbonFileMetastore extends CarbonMetaStore {
 
 
   def isTransactionalCarbonTable(identifier: AbsoluteTableIdentifier): Boolean = {
-    val table = getTableFromMetadataCache(identifier.getDatabaseName, identifier.getTableName)
-    table.map(_.getTableInfo.isTransactionalTable).getOrElse(true)
-  }
-
-  private def getTimestampFileAndType() = {
-    var basePath = CarbonProperties.getInstance()
-      .getProperty(CarbonCommonConstants.CARBON_UPDATE_SYNC_FOLDER,
-        CarbonCommonConstants.CARBON_UPDATE_SYNC_FOLDER_DEFAULT)
-    basePath = CarbonUtil.checkAndAppendFileSystemURIScheme(basePath)
-    val timestampFile = basePath + "/" + CarbonCommonConstants.SCHEMAS_MODIFIED_TIME_FILE
-    val timestampFileType = FileFactory.getFileType(timestampFile)
-    if (!FileFactory.isFileExist(basePath, timestampFileType)) {
-      FileFactory
-        .createDirectoryAndSetPermission(basePath,
-          new FsPermission(FsAction.ALL, FsAction.ALL, FsAction.ALL))
+    val table = Option(CarbonMetadata.getInstance()
+      .getCarbonTable(identifier.getCarbonTableIdentifier.getTableUniqueName))
+    table match {
+      case Some(t) => t.isTransactionalTable
+      case None => true
     }
-    (timestampFile, timestampFileType)
   }
 
   /**
    * This method will put the updated timestamp of schema file in the table modified time store map
-   *
-   * @param timeStamp
    */
-  private def updateSchemasUpdatedTime(timeStamp: Long) {
-    tableModifiedTimeStore.put(CarbonCommonConstants.DATABASE_DEFAULT_NAME, timeStamp)
-  }
-
-  def updateAndTouchSchemasUpdatedTime() {
-    updateSchemasUpdatedTime(touchSchemaFileSystemTime())
+  private def updateSchemasUpdatedTime(tableUniqueId: String, timeStamp: Long) {
+    CarbonFileMetastore.updateTableSchemaModifiedTime(tableUniqueId, timeStamp)
   }
 
 
-  /**
-   * This method will check and create an empty schema timestamp file
-   *
-   * @return
-   */
-  private def touchSchemaFileSystemTime(): Long = {
-    val (timestampFile, timestampFileType) = getTimestampFileAndType()
-    if (!FileFactory.isFileExist(timestampFile, timestampFileType)) {
-      LOGGER.info(s"Creating timestamp file for $timestampFile")
-      FileFactory
-        .createNewFile(timestampFile,
-          timestampFileType,
-          true,
-          new FsPermission(FsAction.ALL, FsAction.ALL, FsAction.ALL))
-    }
-    FileFactory.getCarbonFile(timestampFile, timestampFileType)
-      .setLastModifiedTime(System.currentTimeMillis())
-    // since there is no guarantee that exact same set modified time returns when called
-    // lastmodified time, so better get the time from file.
-    FileFactory.getCarbonFile(timestampFile, timestampFileType)
-      .getLastModifiedTime
-  }
-
-  def checkSchemasModifiedTimeAndReloadTable(tableIdentifier: TableIdentifier): Boolean = {
-    val (timestampFile, timestampFileType) = getTimestampFileAndType()
-    var isRefreshed = false
-    if (FileFactory.isFileExist(timestampFile, timestampFileType)) {
-      val lastModifiedTime =
-        FileFactory.getCarbonFile(timestampFile, timestampFileType).getLastModifiedTime
-      if (!(lastModifiedTime ==
-            tableModifiedTimeStore.get(CarbonCommonConstants.DATABASE_DEFAULT_NAME))) {
-        metadata.writeLock.lock()
-        metadata.carbonTables = metadata.carbonTables.filterNot(
-          table => table.getTableName.equalsIgnoreCase(tableIdentifier.table) &&
-                   table.getDatabaseName
-                     .equalsIgnoreCase(tableIdentifier.database
-                       .getOrElse(SparkSession.getActiveSession.get.sessionState.catalog
-                         .getCurrentDatabase)))
-        metadata.writeLock.unlock()
-        updateSchemasUpdatedTime(lastModifiedTime)
-        isRefreshed = true
-      }
+  override def isSchemaRefreshed(tableUniqueId: String, tablePath: String): Boolean = {
+    val schemaFilePath = CarbonTablePath.getSchemaFilePath(tablePath)
+    val schemaCarbonFile = FileFactory.getCarbonFile(schemaFilePath)
+    val oldTime = Option(CarbonFileMetastore.getTableModifiedTime(tableUniqueId))
+    val newTime = schemaCarbonFile.getLastModifiedTime
+    oldTime match {
+      case Some(cacheTime) =>
+        cacheTime != newTime
+      case None => true
 
 Review comment:
   How another driver updates the latest time into the map.?

----------------------------------------------------------------
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


With regards,
Apache Git Services