Skip to content

fix: reuse a single ZipFile per database during indexing instead of one per LOB #637

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
luis100 wants to merge 1 commit into
base: master
Choose a base branch
from
+85 −28
Open

fix: reuse a single ZipFile per database during indexing instead of one per LOB #637

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
84 changes: 57 additions & 27 deletions ...n/java/com/databasepreservation/common/transformers/ToolkitStructure2ViewerStructure.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -776,8 +776,15 @@ private static ViewerType getType(Type type) throws ViewerException {
return result;
}

/**
* @param sharedZipFile
* an already open ZipFile for databasePath, reused across rows/cells
* to avoid re-reading the ZIP central directory for every LOB (may be
* null, in which case a ZipFile is opened and closed per LOB as
* needed)
*/
public static ViewerRow getRow(CollectionStatus collectionConfiguration, ViewerTable table, Row row, long rowIndex,
String databasePath, String siardVersion) {
String databasePath, String siardVersion, ZipFile sharedZipFile) {
setCurrentTable(table);

ViewerRow result = new ViewerRow();
Expand All @@ -787,7 +794,7 @@ public static ViewerRow getRow(CollectionStatus collectionConfiguration, ViewerT
result.setTableId(table.getId());
result.setTableUUID(table.getUuid());
result.setUuid(rowUUID);
result.setCells(getCells(collectionConfiguration, table, row, databasePath, result, siardVersion));
result.setCells(getCells(collectionConfiguration, table, row, databasePath, result, siardVersion, sharedZipFile));
return result;
}

Expand All @@ -798,7 +805,7 @@ private static void setCurrentTable(ViewerTable table) {
}

private static Map<String, ViewerCell> getCells(CollectionStatus collectionConfiguration, ViewerTable table, Row row,
String databasePath, ViewerRow actualViewerRow, String siardVersion) {
String databasePath, ViewerRow actualViewerRow, String siardVersion, ZipFile sharedZipFile) {
Map<String, ViewerCell> result = new LinkedHashMap<>();

int colIndex = 0;
Expand All @@ -807,7 +814,7 @@ private static Map<String, ViewerCell> getCells(CollectionStatus collectionConfi
String solrColumnName = viewerColumn.getSolrName();
try {
result.put(solrColumnName, getCell(collectionConfiguration, table, toolkitCells.get(colIndex), colIndex++,
databasePath, actualViewerRow, siardVersion));
databasePath, actualViewerRow, siardVersion, sharedZipFile));
} catch (ViewerException e) {
LOGGER.error("Problem converting cell, omitted it (as if it were NULL)", e);
}
Expand All @@ -817,7 +824,8 @@ private static Map<String, ViewerCell> getCells(CollectionStatus collectionConfi
}

private static ViewerCell getCell(CollectionStatus collectionConfiguration, ViewerTable table, Cell cell,
int colIndex, String databasePath, ViewerRow actualViewerRow, String siardVersion) throws ViewerException {
int colIndex, String databasePath, ViewerRow actualViewerRow, String siardVersion, ZipFile sharedZipFile)
throws ViewerException {
ViewerCell result = new ViewerCell();

ViewerType columnType = table.getColumns().get(colIndex).getType();
Expand Down Expand Up @@ -846,7 +854,7 @@ private static ViewerCell getCell(CollectionStatus collectionConfiguration, View

if (!mimeTypeAutoDetectDisable) {
detectMimeType(actualViewerRow, result, databasePath, collectionConfiguration, table, colIndex, lobName,
true);
true, sharedZipFile);
}

} catch (ModuleException e) {
Expand All @@ -870,7 +878,7 @@ private static ViewerCell getCell(CollectionStatus collectionConfiguration, View

if (!mimeTypeAutoDetectDisable) {
detectMimeType(actualViewerRow, result, databasePath, collectionConfiguration, table, colIndex, lobName,
false);
false, sharedZipFile);
}

} else {
Expand All @@ -889,18 +897,18 @@ private static ViewerCell getCell(CollectionStatus collectionConfiguration, View
result.setValue(lobName);
if (!mimeTypeAutoDetectDisable) {
detectMimeType(actualViewerRow, result, databasePath, collectionConfiguration, table, colIndex, lobName,
true, true);
true, true, sharedZipFile);
}
} else {
// Check if LOB is a CLOB
if (columnType.getDbType().equals(ViewerType.dbTypes.CLOB)) {
getCLOBValue(databasePath, binaryCell.getFile()).ifPresent(result::setValue);
getCLOBValue(databasePath, binaryCell.getFile(), sharedZipFile).ifPresent(result::setValue);
} else {
lobName = Paths.get(binaryCell.getFile()).getFileName().toString();
result.setValue(lobName);
if (!mimeTypeAutoDetectDisable) {
detectMimeType(actualViewerRow, result, databasePath, collectionConfiguration, table, colIndex, lobName,
true);
true, sharedZipFile);
}
}
}
Expand Down Expand Up @@ -941,36 +949,51 @@ private static ViewerCell getCell(CollectionStatus collectionConfiguration, View
return result;
}

private static Optional<String> getCLOBValue(String databasePath, String lobName) {

try (ZipFile zipFile = new ZipFile(databasePath)) {
private static Optional<String> getCLOBValue(String databasePath, String lobName, ZipFile sharedZipFile) {
ZipFile zipFile = sharedZipFile;
boolean ownsZipFile = false;
try {
if (zipFile == null) {
zipFile = new ZipFile(databasePath);
ownsZipFile = true;
}
ZipEntry entry = zipFile.getEntry(lobName);
InputStream inputStream = zipFile.getInputStream(entry);
return Optional.of(IOUtils.toString(inputStream, StandardCharsets.UTF_8));
try (InputStream inputStream = zipFile.getInputStream(entry)) {
return Optional.of(IOUtils.toString(inputStream, StandardCharsets.UTF_8));
}
} catch (IOException e) {
LOGGER.error("Failed to obtain CLOB value", e);
} finally {
if (ownsZipFile) {
try {
zipFile.close();
} catch (IOException e) {
LOGGER.error("Failed to close ZIP file", e);
}
}
}
return Optional.empty();
}

private static void detectMimeType(ViewerRow row, ViewerCell cell, String databasePath,
CollectionStatus collectionConfiguration, ViewerTable table, int colIndex, String lobName,
boolean blobIsInsideSiard) {
boolean blobIsInsideSiard, ZipFile sharedZipFile) {
detectMimeType(row, cell, databasePath, collectionConfiguration, table, colIndex, lobName, blobIsInsideSiard,
false);
false, sharedZipFile);
}

private static void detectMimeType(ViewerRow row, ViewerCell cell, String databasePath,
CollectionStatus collectionConfiguration, ViewerTable table, int colIndex, String lobName,
boolean blobIsInsideSiard, boolean isSiardDK) {
boolean blobIsInsideSiard, boolean isSiardDK, ZipFile sharedZipFile) {
ZipFile zipFile = sharedZipFile;
boolean ownsZipFile = false;
InputStream inputStream = null;
try {
String mimeType;
String fileExtension;
InputStream inputStream;

TableStatus tableStatus = collectionConfiguration.getTableStatusByTableId(table.getId());

ZipFile zipFile = null;
ZipEntry entry = null;
String siardLobPath;

Expand All @@ -983,7 +1006,10 @@ private static void detectMimeType(ViewerRow row, ViewerCell cell, String databa

if (!isSiardDK) {
siardLobPath = LobManagerUtils.getZipFilePath(tableStatus, colIndex, lobName);
zipFile = new ZipFile(databasePath);
if (zipFile == null) {
zipFile = new ZipFile(databasePath);
ownsZipFile = true;
}
entry = zipFile.getEntry(siardLobPath);

if (entry != null && blobIsInsideSiard) {
Expand All @@ -1006,6 +1032,7 @@ private static void detectMimeType(ViewerRow row, ViewerCell cell, String databa

if (StringUtils.isAllBlank(fileExtension)) {
try {
IOUtils.closeQuietly(inputStream);
if (blobIsInsideSiard) {
if (entry != null) {
inputStream = zipFile.getInputStream(entry);
Expand Down Expand Up @@ -1036,12 +1063,6 @@ private static void detectMimeType(ViewerRow row, ViewerCell cell, String databa
LOGGER.error("Could not calculate mimeType for special extensions in the cell: [{}]", cell.getValue(), e);
}
}

inputStream.close();

if (zipFile != null) {
zipFile.close();
}
}

cell.setMimeType(mimeType);
Expand All @@ -1057,6 +1078,15 @@ private static void detectMimeType(ViewerRow row, ViewerCell cell, String databa

} catch (IOException | MimeTypeException e) {
LOGGER.error("Could not calculate mimeType for cell: [{}]", cell.getValue(), e);
} finally {
IOUtils.closeQuietly(inputStream);
if (ownsZipFile) {
try {
zipFile.close();
} catch (IOException e) {
LOGGER.error("Failed to close ZIP file", e);
}
}
}
}

Expand Down
29 changes: 28 additions & 1 deletion src/main/java/com/databasepreservation/modules/viewer/DbvtkExportModule.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@
import java.io.IOException;
import java.util.Map;
import java.util.Set;
import java.util.zip.ZipFile;

import org.apache.solr.client.solrj.SolrServerException;
import org.roda.core.data.exceptions.GenericException;
Expand Down Expand Up @@ -47,6 +48,7 @@ public class DbvtkExportModule implements DatabaseFilterModule {
private ViewerTable currentTable;
private String databaseUUID;
private long rowIndex = 1;
private ZipFile databaseZipFile;
private static final Logger LOGGER = LoggerFactory.getLogger(DbvtkExportModule.class);

public DbvtkExportModule(String databaseUUID) {
Expand All @@ -70,6 +72,21 @@ public DbvtkExportModule(String databaseUUID) {
public void initDatabase() throws ModuleException {
LOGGER.info("Starting to process database {}", databaseUUID);
// setup is done when DBVTK starts

// Open the SIARD file once and reuse it for LOB access/mime-type detection
// across all rows and tables, instead of re-opening it (and re-reading its
// whole ZIP central directory) for every LOB cell. Not all SIARD variants
// (e.g. SIARD-DK) are ZIP files, so failing to open it here is expected and
// simply falls back to opening a ZipFile per LOB access.
if (retrieved != null && retrieved.getPath() != null) {
try {
databaseZipFile = new ZipFile(retrieved.getPath());
} catch (IOException e) {
LOGGER.debug("Database at {} could not be opened as a ZIP file, LOB access will open it on demand instead",
retrieved.getPath(), e);
databaseZipFile = null;
}
}
}

/**
Expand Down Expand Up @@ -143,7 +160,7 @@ public void handleDataOpenTable(String tableId) throws ModuleException {
@Override
public void handleDataRow(Row row) throws ModuleException {
solrManager.addRow(collectionConfiguration, ToolkitStructure2ViewerStructure.getRow(collectionConfiguration,
currentTable, row, rowIndex++, retrieved.getPath(), retrieved.getVersion()));
currentTable, row, rowIndex++, retrieved.getPath(), retrieved.getVersion(), databaseZipFile));

rowsProcessedByTableCounter++;
rowCounter++;
Expand Down Expand Up @@ -217,6 +234,16 @@ public void finishDatabase() throws ModuleException {
} catch (IllegalAccessException e) {
throw new ModuleException().withMessage("Error updating collection status").withCause(e);
}

if (databaseZipFile != null) {
try {
databaseZipFile.close();
} catch (IOException e) {
LOGGER.warn("Could not close database ZIP file", e);
}
databaseZipFile = null;
}

LOGGER.info("Finished processing database {}", databaseUUID);
}

Expand Down
Loading