[GitHub] [carbondata] MarvinLitt commented on a change in pull request #3855: [CARBONDATA-3863], after using index service clean the temp data

2020-08-13 Thread GitBox


MarvinLitt commented on a change in pull request #3855:
URL: https://github.com/apache/carbondata/pull/3855#discussion_r469870300



##
File path: 
integration/spark/src/test/scala/org/apache/indexserver/IndexServerTest.scala
##
@@ -0,0 +1,56 @@
+package org.apache.indexserver

Review comment:
   done





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:
us...@infra.apache.org




[GitHub] [carbondata] MarvinLitt commented on a change in pull request #3855: [CARBONDATA-3863], after using index service clean the temp data

2020-08-12 Thread GitBox


MarvinLitt commented on a change in pull request #3855:
URL: https://github.com/apache/carbondata/pull/3855#discussion_r469242044



##
File path: 
integration/spark/src/main/scala/org/apache/carbondata/indexserver/IndexServer.scala
##
@@ -316,4 +324,17 @@ object IndexServer extends ServerInterface {
   Array(new Service("security.indexserver.protocol.acl", 
classOf[ServerInterface]))
 }
   }
+
+  def startAgingFolders(): Unit = {
+val runnable = new Runnable() {
+  def run() {
+val age = System.currentTimeMillis() - agePeriod.toLong
+CarbonUtil.agingTempFolderForIndexServer(age)
+LOGGER.info(s"Complete age temp folder 
${CarbonUtil.getIndexServerTempPath}")
+  }
+}
+val ags: ScheduledExecutorService = 
Executors.newSingleThreadScheduledExecutor
+ags.scheduleAtFixedRate(runnable, 1000, 360, TimeUnit.MICROSECONDS)

Review comment:
   done





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:
us...@infra.apache.org




[GitHub] [carbondata] MarvinLitt commented on a change in pull request #3855: [CARBONDATA-3863], after using index service clean the temp data

2020-08-12 Thread GitBox


MarvinLitt commented on a change in pull request #3855:
URL: https://github.com/apache/carbondata/pull/3855#discussion_r469241896



##
File path: 
core/src/main/java/org/apache/carbondata/core/datastore/filesystem/LocalCarbonFile.java
##
@@ -485,4 +486,30 @@ public boolean equals(Object o) {
   public int hashCode() {
 return Objects.hash(file.getAbsolutePath());
   }
+
+  @Override
+  public List listDirs() throws IOException {
+if (!file.isDirectory()) {
+  return new ArrayList();
+}
+Collection fileCollection = FileUtils.listFilesAndDirs(file,
+DirectoryFileFilter.DIRECTORY, null);
+if (fileCollection.isEmpty()) {
+  return new ArrayList();
+}
+List carbonFiles = new ArrayList();
+for (File file : fileCollection) {
+  if (file.isDirectory()) {
+File[] files = file.listFiles();

Review comment:
   okay,done





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:
us...@infra.apache.org




[GitHub] [carbondata] MarvinLitt commented on a change in pull request #3855: [CARBONDATA-3863], after using index service clean the temp data

2020-08-11 Thread GitBox


MarvinLitt commented on a change in pull request #3855:
URL: https://github.com/apache/carbondata/pull/3855#discussion_r468484427



##
File path: 
integration/spark/src/main/scala/org/apache/carbondata/indexserver/IndexServer.scala
##
@@ -316,4 +324,17 @@ object IndexServer extends ServerInterface {
   Array(new Service("security.indexserver.protocol.acl", 
classOf[ServerInterface]))
 }
   }
+
+  def startAgingFolders(): Unit = {
+val runnable = new Runnable() {
+  def run() {
+val age = System.currentTimeMillis() - agePeriod.toLong
+CarbonUtil.agingTempFolderForIndexServer(age)
+LOGGER.info(s"Complete age temp folder 
${CarbonUtil.getIndexServerTempPath}")
+  }
+}
+val ags: ScheduledExecutorService = 
Executors.newSingleThreadScheduledExecutor
+ags.scheduleAtFixedRate(runnable, 1000, 360, TimeUnit.MICROSECONDS)

Review comment:
   oh sorry will change to milliseconds, how about keep it as 3hours.





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:
us...@infra.apache.org




[GitHub] [carbondata] MarvinLitt commented on a change in pull request #3855: [CARBONDATA-3863], after using index service clean the temp data

2020-07-27 Thread GitBox


MarvinLitt commented on a change in pull request #3855:
URL: https://github.com/apache/carbondata/pull/3855#discussion_r460675486



##
File path: 
integration/spark/src/main/scala/org/apache/carbondata/indexserver/IndexServer.scala
##
@@ -316,4 +324,17 @@ object IndexServer extends ServerInterface {
   Array(new Service("security.indexserver.protocol.acl", 
classOf[ServerInterface]))
 }
   }
+
+  def startAgingFolders(): Unit = {
+val runnable = new Runnable() {
+  def run() {
+val age = System.currentTimeMillis() - agePeriod.toLong
+CarbonUtil.agingTempFolderForIndexServer(age)
+LOGGER.info(s"Complete age temp folder 
${CarbonUtil.getIndexServerTempPath}")
+  }
+}
+val ags: ScheduledExecutorService = 
Executors.newSingleThreadScheduledExecutor
+ags.scheduleAtFixedRate(runnable, 1000, 360, TimeUnit.MICROSECONDS)

Review comment:
   the rate is 3 hours, about the delay time, i thinks it is ok, delay 1s 
or delay 5min or delay 1hour the effect is almost the same.
   The test cases are covered here. If there is too much delay, the execution 
of test cases will be affected.
   so kunal is there no need to modify the delay here?





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:
us...@infra.apache.org




[GitHub] [carbondata] MarvinLitt commented on a change in pull request #3855: [CARBONDATA-3863], after using index service clean the temp data

2020-07-27 Thread GitBox


MarvinLitt commented on a change in pull request #3855:
URL: https://github.com/apache/carbondata/pull/3855#discussion_r460667653



##
File path: 
integration/spark/src/main/scala/org/apache/carbondata/indexserver/IndexServer.scala
##
@@ -316,4 +324,17 @@ object IndexServer extends ServerInterface {
   Array(new Service("security.indexserver.protocol.acl", 
classOf[ServerInterface]))
 }
   }
+
+  def startAgingFolders(): Unit = {

Review comment:
   done





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:
us...@infra.apache.org




[GitHub] [carbondata] MarvinLitt commented on a change in pull request #3855: [CARBONDATA-3863], after using index service clean the temp data

2020-07-27 Thread GitBox


MarvinLitt commented on a change in pull request #3855:
URL: https://github.com/apache/carbondata/pull/3855#discussion_r460666952



##
File path: 
integration/spark/src/main/scala/org/apache/carbondata/indexserver/IndexServer.scala
##
@@ -316,4 +324,17 @@ object IndexServer extends ServerInterface {
   Array(new Service("security.indexserver.protocol.acl", 
classOf[ServerInterface]))
 }
   }
+
+  def startAgingFolders(): Unit = {
+val runnable = new Runnable() {
+  def run() {
+val age = System.currentTimeMillis() - agePeriod.toLong
+CarbonUtil.agingTempFolderForIndexServer(age)
+LOGGER.info(s"Complete age temp folder 
${CarbonUtil.getIndexServerTempPath}")
+  }
+}
+val ags: ScheduledExecutorService = 
Executors.newSingleThreadScheduledExecutor
+ags.scheduleAtFixedRate(runnable, 1000, 360, TimeUnit.MICROSECONDS)
+LOGGER.info("index server temp folders aging thread start")

Review comment:
   under run func there already has logs.
 def run() {
   val age = System.currentTimeMillis() - agePeriod.toLong
   CarbonUtil.agingTempFolderForIndexServer(age)
   LOGGER.info(s"Complete age temp folder 
${CarbonUtil.getIndexServerTempPath}")
 }





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:
us...@infra.apache.org




[GitHub] [carbondata] MarvinLitt commented on a change in pull request #3855: [CARBONDATA-3863], after using index service clean the temp data

2020-07-24 Thread GitBox


MarvinLitt commented on a change in pull request #3855:
URL: https://github.com/apache/carbondata/pull/3855#discussion_r460346871



##
File path: 
core/src/main/java/org/apache/carbondata/core/datastore/filesystem/LocalCarbonFile.java
##
@@ -485,4 +486,30 @@ public boolean equals(Object o) {
   public int hashCode() {
 return Objects.hash(file.getAbsolutePath());
   }
+
+  @Override
+  public List listDirs() throws IOException {
+if (!file.isDirectory()) {
+  return new ArrayList();
+}
+Collection fileCollection = FileUtils.listFilesAndDirs(file,
+DirectoryFileFilter.DIRECTORY, null);
+if (fileCollection.isEmpty()) {
+  return new ArrayList();
+}
+List carbonFiles = new ArrayList();
+for (File file : fileCollection) {
+  if (file.isDirectory()) {
+File[] files = file.listFiles();

Review comment:
   try to find all folders from  /tmp/indexservertmp.
   no  list recursive, just find the direct directory under /tmp/indexservertmp.





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:
us...@infra.apache.org




[GitHub] [carbondata] MarvinLitt commented on a change in pull request #3855: [CARBONDATA-3863], after using index service clean the temp data

2020-07-24 Thread GitBox


MarvinLitt commented on a change in pull request #3855:
URL: https://github.com/apache/carbondata/pull/3855#discussion_r460346573



##
File path: 
integration/spark/src/main/scala/org/apache/carbondata/indexserver/IndexServer.scala
##
@@ -316,4 +324,17 @@ object IndexServer extends ServerInterface {
   Array(new Service("security.indexserver.protocol.acl", 
classOf[ServerInterface]))
 }
   }
+
+  def startAgingFolders(): Unit = {
+val runnable = new Runnable() {
+  def run() {
+val age = System.currentTimeMillis() - agePeriod.toLong
+CarbonUtil.agingTempFolderForIndexServer(age)
+LOGGER.info(s"Complete age temp folder 
${CarbonUtil.getIndexServerTempPath}")
+  }
+}
+val ags: ScheduledExecutorService = 
Executors.newSingleThreadScheduledExecutor
+ags.scheduleAtFixedRate(runnable, 1000, 360, TimeUnit.MICROSECONDS)

Review comment:
   delay 1 hour may be to late, if something wrong happens, 1hour is enough 
to cause a relatively large impcact。
   may be we can delay for 5 min, what do you think? 





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:
us...@infra.apache.org




[GitHub] [carbondata] MarvinLitt commented on a change in pull request #3855: [CARBONDATA-3863], after using index service clean the temp data

2020-07-24 Thread GitBox


MarvinLitt commented on a change in pull request #3855:
URL: https://github.com/apache/carbondata/pull/3855#discussion_r460346175



##
File path: 
integration/spark/src/main/scala/org/apache/carbondata/indexserver/IndexJobs.scala
##
@@ -69,6 +69,12 @@ class DistributedIndexJob extends AbstractIndexJob {
 if (null != splitFolderPath && !splitFolderPath.deleteFile()) {
   LOGGER.error("Problem while deleting the temp directory:"
 + splitFolderPath.getAbsolutePath)
+} else {
+  // if the path build with getQueryId already exists,
+  // the splitFolderPath should be null, need delete
+  if (null == splitFolderPath) {
+CarbonUtil.deleteTempFolderForIndexServer(indexFormat.getQueryId)

Review comment:
   if the /tmp/indexservertmp/gropid  has exists then the splitFolderPath 
will return null.
   before our code do not deal with that scene。





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:
us...@infra.apache.org




[GitHub] [carbondata] MarvinLitt commented on a change in pull request #3855: [CARBONDATA-3863], after using index service clean the temp data

2020-07-24 Thread GitBox


MarvinLitt commented on a change in pull request #3855:
URL: https://github.com/apache/carbondata/pull/3855#discussion_r460346175



##
File path: 
integration/spark/src/main/scala/org/apache/carbondata/indexserver/IndexJobs.scala
##
@@ -69,6 +69,12 @@ class DistributedIndexJob extends AbstractIndexJob {
 if (null != splitFolderPath && !splitFolderPath.deleteFile()) {
   LOGGER.error("Problem while deleting the temp directory:"
 + splitFolderPath.getAbsolutePath)
+} else {
+  // if the path build with getQueryId already exists,
+  // the splitFolderPath should be null, need delete
+  if (null == splitFolderPath) {
+CarbonUtil.deleteTempFolderForIndexServer(indexFormat.getQueryId)

Review comment:
   if the /tmp/indexservertmp/gropid  has exists then the splitFolderPath 
will return null.
   before our code do not deal with that scene。
   found this issue by test case.





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:
us...@infra.apache.org