This is an automated email from the ASF dual-hosted git repository.
mayanks pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/pinot.git
The following commit(s) were added to refs/heads/master by this push:
new 823aa07d delete tmp- segment directories on server startup (#7961)
823aa07d is described below
commit 823aa07d7fc61543ed3d91992749d28aecd192ee
Author: Johan Adami <[email protected]>
AuthorDate: Mon Jan 10 19:50:22 2022 -0500
delete tmp- segment directories on server startup (#7961)
* delete tmp- segment directories on server startup
* no need to sleep 100
* add license
* rename dataDir for checkstyle
* java 8 compatible
* sleep longer to ensure later mod time
* setLastModified directly
* rename deletion function; use 3 hours before app start time
* rename config
* move all usages of tmp to a dedicated directory
* fix imports and checkstyle
* check if tmp dir exists; create actual dir in
BaseTableDataManagerAcquireSegmentTest
* revert segment manager changes; add back comment; try to fix test
* fix convert raw index test
* deleteQuietly; add comment
Co-authored-by: Johan Adami <[email protected]>
---
.../pinot/core/data/manager/BaseTableDataManager.java | 15 ++++++++++++++-
.../manager/realtime/RealtimeTableDataManager.java | 6 +++---
.../BaseTableDataManagerAcquireSegmentTest.java | 5 ++++-
.../core/data/manager/BaseTableDataManagerTest.java | 19 ++++++++++---------
...ConvertToRawIndexMinionClusterIntegrationTest.java | 12 ++++++++++--
5 files changed, 41 insertions(+), 16 deletions(-)
diff --git
a/pinot-core/src/main/java/org/apache/pinot/core/data/manager/BaseTableDataManager.java
b/pinot-core/src/main/java/org/apache/pinot/core/data/manager/BaseTableDataManager.java
index 43f4d9c..1a5c013 100644
---
a/pinot-core/src/main/java/org/apache/pinot/core/data/manager/BaseTableDataManager.java
+++
b/pinot-core/src/main/java/org/apache/pinot/core/data/manager/BaseTableDataManager.java
@@ -75,6 +75,7 @@ public abstract class BaseTableDataManager implements
TableDataManager {
protected String _tableNameWithType;
protected String _tableDataDir;
protected File _indexDir;
+ protected File _resourceTmpDir;
protected Logger _logger;
protected HelixManager _helixManager;
protected String _authToken;
@@ -102,6 +103,13 @@ public abstract class BaseTableDataManager implements
TableDataManager {
if (!_indexDir.exists()) {
Preconditions.checkState(_indexDir.mkdirs());
}
+ _resourceTmpDir = new File(_indexDir, "tmp");
+ // This is meant to cleanup temp resources from TableDataManager. But
other code using this same
+ // directory will have those deleted as well.
+ FileUtils.deleteQuietly(_resourceTmpDir);
+ if (!_resourceTmpDir.exists()) {
+ Preconditions.checkState(_resourceTmpDir.mkdirs());
+ }
_errorCache = errorCache;
_logger = LoggerFactory.getLogger(_tableNameWithType + "-" +
getClass().getSimpleName());
@@ -423,7 +431,7 @@ public abstract class BaseTableDataManager implements
TableDataManager {
private File downloadSegmentFromDeepStore(String segmentName,
SegmentZKMetadata zkMetadata)
throws Exception {
- File tempRootDir = getSegmentDataDir("tmp-" + segmentName + "-" +
UUID.randomUUID());
+ File tempRootDir = getTmpSegmentDataDir("tmp-" + segmentName + "-" +
UUID.randomUUID());
FileUtils.forceMkdir(tempRootDir);
try {
File tarFile = downloadAndDecrypt(segmentName, zkMetadata, tempRootDir);
@@ -481,6 +489,11 @@ public abstract class BaseTableDataManager implements
TableDataManager {
}
@VisibleForTesting
+ protected File getTmpSegmentDataDir(String segmentName) {
+ return new File(_resourceTmpDir, segmentName);
+ }
+
+ @VisibleForTesting
static boolean isNewSegment(SegmentZKMetadata zkMetadata, @Nullable
SegmentMetadata localMetadata) {
return localMetadata == null || !hasSameCRC(zkMetadata, localMetadata);
}
diff --git
a/pinot-core/src/main/java/org/apache/pinot/core/data/manager/realtime/RealtimeTableDataManager.java
b/pinot-core/src/main/java/org/apache/pinot/core/data/manager/realtime/RealtimeTableDataManager.java
index 68b31c4..b74a9d7 100644
---
a/pinot-core/src/main/java/org/apache/pinot/core/data/manager/realtime/RealtimeTableDataManager.java
+++
b/pinot-core/src/main/java/org/apache/pinot/core/data/manager/realtime/RealtimeTableDataManager.java
@@ -465,9 +465,9 @@ public class RealtimeTableDataManager extends
BaseTableDataManager {
*/
private void untarAndMoveSegment(String segmentName, IndexLoadingConfig
indexLoadingConfig, File segmentTarFile)
throws IOException {
- // TODO: This could leave temporary directories in _indexDir if JVM shuts
down before the temporary directory is
- // deleted. Consider cleaning up all temporary directories when
starting the server.
- File tempSegmentDir = new File(_indexDir, "tmp-" + segmentName + "." +
System.currentTimeMillis());
+ // This could leave temporary directories in _indexDir if JVM shuts down
before the temp directory is deleted.
+ // This is fine since the temporary directories are deleted when the table
data manager calls init.
+ File tempSegmentDir = getTmpSegmentDataDir("tmp-" + segmentName + "." +
System.currentTimeMillis());
try {
File tempIndexDir = TarGzCompressionUtils.untar(segmentTarFile,
tempSegmentDir).get(0);
_logger.info("Uncompressed file {} into tmp dir {}", segmentTarFile,
tempSegmentDir);
diff --git
a/pinot-core/src/test/java/org/apache/pinot/core/data/manager/BaseTableDataManagerAcquireSegmentTest.java
b/pinot-core/src/test/java/org/apache/pinot/core/data/manager/BaseTableDataManagerAcquireSegmentTest.java
index 9c03d51..43be64b 100644
---
a/pinot-core/src/test/java/org/apache/pinot/core/data/manager/BaseTableDataManagerAcquireSegmentTest.java
+++
b/pinot-core/src/test/java/org/apache/pinot/core/data/manager/BaseTableDataManagerAcquireSegmentTest.java
@@ -29,6 +29,7 @@ import java.util.Random;
import java.util.Set;
import java.util.concurrent.ConcurrentHashMap;
import java.util.concurrent.atomic.AtomicInteger;
+import org.apache.commons.io.FileUtils;
import org.apache.helix.HelixManager;
import org.apache.helix.store.zk.ZkHelixPropertyStore;
import org.apache.pinot.common.metrics.PinotMetricUtils;
@@ -40,6 +41,7 @@ import
org.apache.pinot.segment.local.data.manager.TableDataManager;
import org.apache.pinot.segment.local.data.manager.TableDataManagerConfig;
import org.apache.pinot.segment.spi.ImmutableSegment;
import org.apache.pinot.segment.spi.SegmentMetadata;
+import org.apache.pinot.util.TestUtils;
import org.testng.Assert;
import org.testng.annotations.AfterSuite;
import org.testng.annotations.BeforeMethod;
@@ -79,7 +81,8 @@ public class BaseTableDataManagerAcquireSegmentTest {
@BeforeSuite
public void setUp()
throws Exception {
- _tmpDir = File.createTempFile("OfflineTableDataManagerTest", null);
+ _tmpDir = new File(FileUtils.getTempDirectory(),
"OfflineTableDataManagerTest");
+ TestUtils.ensureDirectoriesExistAndEmpty(_tmpDir);
_tmpDir.deleteOnExit();
long seed = System.currentTimeMillis();
diff --git
a/pinot-core/src/test/java/org/apache/pinot/core/data/manager/BaseTableDataManagerTest.java
b/pinot-core/src/test/java/org/apache/pinot/core/data/manager/BaseTableDataManagerTest.java
index 1a5a3d5..78af028 100644
---
a/pinot-core/src/test/java/org/apache/pinot/core/data/manager/BaseTableDataManagerTest.java
+++
b/pinot-core/src/test/java/org/apache/pinot/core/data/manager/BaseTableDataManagerTest.java
@@ -91,7 +91,7 @@ public class BaseTableDataManagerTest {
public void testReloadSegmentNewData()
throws Exception {
BaseTableDataManager tmgr = makeTestableManager();
- File tempRootDir = tmgr.getSegmentDataDir("test-new-data");
+ File tempRootDir = tmgr.getTmpSegmentDataDir("test-new-data");
// Create an empty segment and compress it to tar.gz as the one in deep
store.
// All input and intermediate files are put in the tempRootDir.
@@ -124,7 +124,7 @@ public class BaseTableDataManagerTest {
public void testReloadSegmentLocalCopy()
throws Exception {
BaseTableDataManager tmgr = makeTestableManager();
- File tempRootDir = tmgr.getSegmentDataDir("test-local-copy");
+ File tempRootDir = tmgr.getTmpSegmentDataDir("test-local-copy");
// Create an empty segment and compress it to tar.gz as the one in deep
store.
// All input and intermediate files are put in the tempRootDir.
@@ -157,7 +157,7 @@ public class BaseTableDataManagerTest {
public void testReloadSegmentForceDownload()
throws Exception {
BaseTableDataManager tmgr = makeTestableManager();
- File tempRootDir = tmgr.getSegmentDataDir("test-force-download");
+ File tempRootDir = tmgr.getTmpSegmentDataDir("test-force-download");
// Create an empty segment and compress it to tar.gz as the one in deep
store.
// All input and intermediate files are put in the tempRootDir.
@@ -190,7 +190,7 @@ public class BaseTableDataManagerTest {
public void testAddOrReplaceSegmentNewData()
throws Exception {
BaseTableDataManager tmgr = makeTestableManager();
- File tempRootDir = tmgr.getSegmentDataDir("test-new-data");
+ File tempRootDir = tmgr.getTmpSegmentDataDir("test-new-data");
// Create an empty segment and compress it to tar.gz as the one in deep
store.
// All input and intermediate files are put in the tempRootDir.
@@ -255,7 +255,7 @@ public class BaseTableDataManagerTest {
public void testAddOrReplaceSegmentNotRecovered()
throws Exception {
BaseTableDataManager tmgr = makeTestableManager();
- File tempRootDir = tmgr.getSegmentDataDir("test-force-download");
+ File tempRootDir = tmgr.getTmpSegmentDataDir("test-force-download");
// Create an empty segment and compress it to tar.gz as the one in deep
store.
// All input and intermediate files are put in the tempRootDir.
@@ -291,7 +291,7 @@ public class BaseTableDataManagerTest {
when(zkmd.getDownloadUrl()).thenReturn("file://" +
tempInput.getAbsolutePath());
BaseTableDataManager tmgr = makeTestableManager();
- File tempRootDir = tmgr.getSegmentDataDir("test-download-decrypt");
+ File tempRootDir = tmgr.getTmpSegmentDataDir("test-download-decrypt");
File tarFile = tmgr.downloadAndDecrypt("seg01", zkmd, tempRootDir);
assertEquals(FileUtils.readFileToString(tarFile), "this is from somewhere
remote");
@@ -301,8 +301,9 @@ public class BaseTableDataManagerTest {
assertEquals(FileUtils.readFileToString(tarFile), "this is from somewhere
remote");
FakePinotCrypter fakeCrypter = (FakePinotCrypter)
PinotCrypterFactory.create("fakePinotCrypter");
-
assertTrue(fakeCrypter._origFile.getAbsolutePath().endsWith("__table01__/test-download-decrypt/seg01.tar.gz.enc"));
-
assertTrue(fakeCrypter._decFile.getAbsolutePath().endsWith("__table01__/test-download-decrypt/seg01.tar.gz"));
+ assertTrue(
+
fakeCrypter._origFile.getAbsolutePath().endsWith("__table01__/tmp/test-download-decrypt/seg01.tar.gz.enc"));
+
assertTrue(fakeCrypter._decFile.getAbsolutePath().endsWith("__table01__/tmp/test-download-decrypt/seg01.tar.gz"));
try {
// Set maxRetry to 0 to cause retry failure immediately.
@@ -320,7 +321,7 @@ public class BaseTableDataManagerTest {
public void testUntarAndMoveSegment()
throws IOException {
BaseTableDataManager tmgr = makeTestableManager();
- File tempRootDir = tmgr.getSegmentDataDir("test-untar-move");
+ File tempRootDir = tmgr.getTmpSegmentDataDir("test-untar-move");
// All input and intermediate files are put in the tempRootDir.
File tempTar = new File(tempRootDir, "seg01" +
TarGzCompressionUtils.TAR_GZ_FILE_EXTENSION);
diff --git
a/pinot-integration-tests/src/test/java/org/apache/pinot/integration/tests/ConvertToRawIndexMinionClusterIntegrationTest.java
b/pinot-integration-tests/src/test/java/org/apache/pinot/integration/tests/ConvertToRawIndexMinionClusterIntegrationTest.java
index ba58aea..b1df10a 100644
---
a/pinot-integration-tests/src/test/java/org/apache/pinot/integration/tests/ConvertToRawIndexMinionClusterIntegrationTest.java
+++
b/pinot-integration-tests/src/test/java/org/apache/pinot/integration/tests/ConvertToRawIndexMinionClusterIntegrationTest.java
@@ -19,6 +19,7 @@
package org.apache.pinot.integration.tests;
import java.io.File;
+import java.io.FilenameFilter;
import java.util.Arrays;
import java.util.Collections;
import java.util.HashMap;
@@ -99,7 +100,14 @@ public class ConvertToRawIndexMinionClusterIntegrationTest
extends HybridCluster
File tableDataDir = testDataDir;
// Check that all columns have dictionary
- File[] indexDirs = tableDataDir.listFiles();
+ // Skip the tmp directory since these are only temporary segments
+ FilenameFilter nonTmpFileFilter = new FilenameFilter() {
+ @Override
+ public boolean accept(File dir, String name) {
+ return !name.equals("tmp");
+ }
+ };
+ File[] indexDirs = tableDataDir.listFiles(nonTmpFileFilter);
Assert.assertNotNull(indexDirs);
for (File indexDir : indexDirs) {
SegmentMetadata segmentMetadata = new SegmentMetadataImpl(indexDir);
@@ -138,7 +146,7 @@ public class ConvertToRawIndexMinionClusterIntegrationTest
extends HybridCluster
}
// Check segment metadata
- File[] indexDirs1 = tableDataDir.listFiles();
+ File[] indexDirs1 = tableDataDir.listFiles(nonTmpFileFilter);
Assert.assertNotNull(indexDirs1);
for (File indexDir : indexDirs1) {
SegmentMetadata segmentMetadata;
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
