Skip to content

Commit

Permalink
cleanup temp files for nested column serializer (apache#15236) (apach…
Browse files Browse the repository at this point in the history 
…e#15246)
  • Loading branch information
@clintropolis
clintropolis authored Oct 25, 2023
1 parent 701b9af commit 0760e8b
Show file tree
Hide file tree
Showing 10 changed files with 282 additions and 53 deletions.
21 changes: 15 additions & 6 deletions processing/src/main/java/org/apache/druid/java/util/common/FileUtils.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -430,33 +430,42 @@ public static File createTempDir()
*
* @throws IllegalStateException if the directory could not be created
*/
@SuppressForbidden(reason = "Files#createTempDirectory")
public static File createTempDir(@Nullable final String prefix)
{
return createTempDirInLocation(getTempDir(), prefix);
}

public static Path getTempDir()
{
final String parentDirectory = System.getProperty("java.io.tmpdir");

if (parentDirectory == null) {
// Not expected.
throw new ISE("System property java.io.tmpdir is not set, cannot create temporary directories");
}
return new File(parentDirectory).toPath();
}

@SuppressForbidden(reason = "Files#createTempDirectory")
public static File createTempDirInLocation(final Path parentDirectory, @Nullable final String prefix)
{
try {
final Path tmpPath = Files.createTempDirectory(
new File(parentDirectory).toPath(),
parentDirectory,
prefix == null || prefix.isEmpty() ? "druid" : prefix
);
return tmpPath.toFile();
}
catch (IOException e) {
// Some inspection to improve error messages.
if (e instanceof NoSuchFileException && !new File(parentDirectory).exists()) {
throw new ISE("java.io.tmpdir (%s) does not exist", parentDirectory);
if (e instanceof NoSuchFileException && !parentDirectory.toFile().exists()) {
throw new ISE("Path [%s] does not exist", parentDirectory);
} else if ((e instanceof FileSystemException && e.getMessage().contains("Read-only file system"))
|| (e instanceof AccessDeniedException)) {
throw new ISE("java.io.tmpdir (%s) is not writable, check permissions", parentDirectory);
throw new ISE("Path [%s] is not writable, check permissions", parentDirectory);
} else {
// Well, maybe it was something else.
throw new ISE(e, "Failed to create temporary directory in java.io.tmpdir (%s)", parentDirectory);
throw new ISE(e, "Failed to create temporary directory in path [%s]", parentDirectory);
}
}
}
Expand Down
57 changes: 44 additions & 13 deletions processing/src/main/java/org/apache/druid/segment/nested/DictionaryIdLookup.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -56,35 +56,43 @@
public final class DictionaryIdLookup implements Closeable
{
private final String name;
private final Path tempBasePath;

@Nullable
private final DictionaryWriter<String> stringDictionaryWriter;
private Path stringDictionaryFile = null;
private SmooshedFileMapper stringBufferMapper = null;
private Indexed<ByteBuffer> stringDictionary = null;

@Nullable
private final DictionaryWriter<Long> longDictionaryWriter;
private Path longDictionaryFile = null;
private MappedByteBuffer longBuffer = null;
private FixedIndexed<Long> longDictionary = null;

@Nullable
private final DictionaryWriter<Double> doubleDictionaryWriter;
private Path doubleDictionaryFile = null;
MappedByteBuffer doubleBuffer = null;
FixedIndexed<Double> doubleDictionary = null;

@Nullable
private final DictionaryWriter<int[]> arrayDictionaryWriter;
private Path arrayDictionaryFile = null;
private MappedByteBuffer arrayBuffer = null;
private FrontCodedIntArrayIndexed arrayDictionary = null;

public DictionaryIdLookup(
String name,
Path tempBasePath,
@Nullable DictionaryWriter<String> stringDictionaryWriter,
@Nullable DictionaryWriter<Long> longDictionaryWriter,
@Nullable DictionaryWriter<Double> doubleDictionaryWriter,
@Nullable DictionaryWriter<int[]> arrayDictionaryWriter
)
{
this.name = name;
this.tempBasePath = tempBasePath;
this.stringDictionaryWriter = stringDictionaryWriter;
this.longDictionaryWriter = longDictionaryWriter;
this.doubleDictionaryWriter = doubleDictionaryWriter;
Expand All @@ -98,16 +106,20 @@ public int lookupString(@Nullable String value)
// for strings because of this. if other type dictionary writers could potentially use multiple internal files
// in the future, we should transition them to using this approach as well (or build a combination smoosher and
// mapper so that we can have a mutable smoosh)
File stringSmoosh = FileUtils.createTempDir(StringUtils.urlEncode(name) + "__stringTempSmoosh");
File stringSmoosh = FileUtils.createTempDirInLocation(tempBasePath, StringUtils.urlEncode(name) + "__stringTempSmoosh");
stringDictionaryFile = stringSmoosh.toPath();
final String fileName = NestedCommonFormatColumnSerializer.getInternalFileName(
name,
NestedCommonFormatColumnSerializer.STRING_DICTIONARY_FILE_NAME
);
final FileSmoosher smoosher = new FileSmoosher(stringSmoosh);
try (final SmooshedWriter writer = smoosher.addWithSmooshedWriter(
fileName,
stringDictionaryWriter.getSerializedSize()
)) {

try (
final FileSmoosher smoosher = new FileSmoosher(stringSmoosh);
final SmooshedWriter writer = smoosher.addWithSmooshedWriter(
fileName,
stringDictionaryWriter.getSerializedSize()
)
) {
stringDictionaryWriter.writeTo(writer, smoosher);
writer.close();
smoosher.close();
Expand All @@ -134,8 +146,8 @@ public int lookupString(@Nullable String value)
public int lookupLong(@Nullable Long value)
{
if (longDictionary == null) {
final Path longFile = makeTempFile(name + NestedCommonFormatColumnSerializer.LONG_DICTIONARY_FILE_NAME);
longBuffer = mapWriter(longFile, longDictionaryWriter);
longDictionaryFile = makeTempFile(name + NestedCommonFormatColumnSerializer.LONG_DICTIONARY_FILE_NAME);
longBuffer = mapWriter(longDictionaryFile, longDictionaryWriter);
longDictionary = FixedIndexed.read(longBuffer, TypeStrategies.LONG, ByteOrder.nativeOrder(), Long.BYTES).get();
// reset position
longBuffer.position(0);
Expand All @@ -150,8 +162,8 @@ public int lookupLong(@Nullable Long value)
public int lookupDouble(@Nullable Double value)
{
if (doubleDictionary == null) {
final Path doubleFile = makeTempFile(name + NestedCommonFormatColumnSerializer.DOUBLE_DICTIONARY_FILE_NAME);
doubleBuffer = mapWriter(doubleFile, doubleDictionaryWriter);
doubleDictionaryFile = makeTempFile(name + NestedCommonFormatColumnSerializer.DOUBLE_DICTIONARY_FILE_NAME);
doubleBuffer = mapWriter(doubleDictionaryFile, doubleDictionaryWriter);
doubleDictionary = FixedIndexed.read(
doubleBuffer,
TypeStrategies.DOUBLE,
Expand All @@ -171,8 +183,8 @@ public int lookupDouble(@Nullable Double value)
public int lookupArray(@Nullable int[] value)
{
if (arrayDictionary == null) {
final Path arrayFile = makeTempFile(name + NestedCommonFormatColumnSerializer.ARRAY_DICTIONARY_FILE_NAME);
arrayBuffer = mapWriter(arrayFile, arrayDictionaryWriter);
arrayDictionaryFile = makeTempFile(name + NestedCommonFormatColumnSerializer.ARRAY_DICTIONARY_FILE_NAME);
arrayBuffer = mapWriter(arrayDictionaryFile, arrayDictionaryWriter);
arrayDictionary = FrontCodedIntArrayIndexed.read(arrayBuffer, ByteOrder.nativeOrder()).get();
// reset position
arrayBuffer.position(0);
Expand Down Expand Up @@ -213,15 +225,19 @@ public void close()
{
if (stringBufferMapper != null) {
stringBufferMapper.close();
deleteTempFile(stringDictionaryFile);
}
if (longBuffer != null) {
ByteBufferUtils.unmap(longBuffer);
deleteTempFile(longDictionaryFile);
}
if (doubleBuffer != null) {
ByteBufferUtils.unmap(doubleBuffer);
deleteTempFile(doubleDictionaryFile);
}
if (arrayBuffer != null) {
ByteBufferUtils.unmap(arrayBuffer);
deleteTempFile(arrayDictionaryFile);
}
}

Expand All @@ -243,7 +259,22 @@ private int arrayOffset()
private Path makeTempFile(String name)
{
try {
return Files.createTempFile(StringUtils.urlEncode(name), null);
return Files.createTempFile(tempBasePath, StringUtils.urlEncode(name), null);
}
catch (IOException e) {
throw new RuntimeException(e);
}
}

private void deleteTempFile(Path path)
{
try {
final File file = path.toFile();
if (file.isDirectory()) {
FileUtils.deleteDirectory(file);
} else {
Files.delete(path);
}
}
catch (IOException e) {
throw new RuntimeException(e);
Expand Down
2 changes: 2 additions & 0 deletions processing/src/main/java/org/apache/druid/segment/nested/NestedDataColumnSerializer.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@
import org.apache.druid.collections.bitmap.ImmutableBitmap;
import org.apache.druid.collections.bitmap.MutableBitmap;
import org.apache.druid.common.config.NullHandling;
import org.apache.druid.java.util.common.FileUtils;
import org.apache.druid.java.util.common.ISE;
import org.apache.druid.java.util.common.RE;
import org.apache.druid.java.util.common.io.Closer;
Expand Down Expand Up @@ -234,6 +235,7 @@ public void openDictionaryWriter() throws IOException
globalDictionaryIdLookup = closer.register(
new DictionaryIdLookup(
name,
FileUtils.getTempDir(),
dictionaryWriter,
longDictionaryWriter,
doubleDictionaryWriter,
Expand Down
2 changes: 2 additions & 0 deletions processing/src/main/java/org/apache/druid/segment/nested/NestedDataColumnSerializerV4.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@
import org.apache.druid.collections.bitmap.ImmutableBitmap;
import org.apache.druid.collections.bitmap.MutableBitmap;
import org.apache.druid.common.config.NullHandling;
import org.apache.druid.java.util.common.FileUtils;
import org.apache.druid.java.util.common.ISE;
import org.apache.druid.java.util.common.RE;
import org.apache.druid.java.util.common.StringUtils;
Expand Down Expand Up @@ -198,6 +199,7 @@ public void open() throws IOException
globalDictionaryIdLookup = closer.register(
new DictionaryIdLookup(
name,
FileUtils.getTempDir(),
dictionaryWriter,
longDictionaryWriter,
doubleDictionaryWriter,
Expand Down
2 changes: 2 additions & 0 deletions processing/src/main/java/org/apache/druid/segment/nested/ScalarDoubleColumnSerializer.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@

package org.apache.druid.segment.nested;

import org.apache.druid.java.util.common.FileUtils;
import org.apache.druid.java.util.common.ISE;
import org.apache.druid.java.util.common.StringUtils;
import org.apache.druid.java.util.common.io.Closer;
Expand Down Expand Up @@ -76,6 +77,7 @@ public void openDictionaryWriter() throws IOException
dictionaryIdLookup = closer.register(
new DictionaryIdLookup(
name,
FileUtils.getTempDir(),
null,
null,
dictionaryWriter,
Expand Down
2 changes: 2 additions & 0 deletions processing/src/main/java/org/apache/druid/segment/nested/ScalarLongColumnSerializer.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@

package org.apache.druid.segment.nested;

import org.apache.druid.java.util.common.FileUtils;
import org.apache.druid.java.util.common.ISE;
import org.apache.druid.java.util.common.StringUtils;
import org.apache.druid.java.util.common.io.Closer;
Expand Down Expand Up @@ -77,6 +78,7 @@ public void openDictionaryWriter() throws IOException
dictionaryIdLookup = closer.register(
new DictionaryIdLookup(
name,
FileUtils.getTempDir(),
null,
dictionaryWriter,
null,
Expand Down
2 changes: 2 additions & 0 deletions processing/src/main/java/org/apache/druid/segment/nested/ScalarStringColumnSerializer.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@
package org.apache.druid.segment.nested;

import org.apache.druid.common.config.NullHandling;
import org.apache.druid.java.util.common.FileUtils;
import org.apache.druid.java.util.common.ISE;
import org.apache.druid.java.util.common.io.Closer;
import org.apache.druid.java.util.common.io.smoosh.FileSmoosher;
Expand Down Expand Up @@ -71,6 +72,7 @@ public void openDictionaryWriter() throws IOException
dictionaryIdLookup = closer.register(
new DictionaryIdLookup(
name,
FileUtils.getTempDir(),
dictionaryWriter,
null,
null,
Expand Down
2 changes: 2 additions & 0 deletions processing/src/main/java/org/apache/druid/segment/nested/VariantColumnSerializer.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@
import org.apache.druid.collections.bitmap.ImmutableBitmap;
import org.apache.druid.collections.bitmap.MutableBitmap;
import org.apache.druid.common.config.NullHandling;
import org.apache.druid.java.util.common.FileUtils;
import org.apache.druid.java.util.common.ISE;
import org.apache.druid.java.util.common.StringUtils;
import org.apache.druid.java.util.common.io.Closer;
Expand Down Expand Up @@ -154,6 +155,7 @@ public void openDictionaryWriter() throws IOException
dictionaryIdLookup = closer.register(
new DictionaryIdLookup(
name,
FileUtils.getTempDir(),
dictionaryWriter,
longDictionaryWriter,
doubleDictionaryWriter,
Expand Down
Loading

0 comments on commit 0760e8b

Please sign in to comment.