From 5e54238580488d9e067387e4db0fa2489209a506 Mon Sep 17 00:00:00 2001 From: "sergio.ramazzina" Date: Mon, 10 Jan 2022 19:45:39 +0100 Subject: [PATCH 1/3] HOP-3673 Connections not closed if a pipeline is run with transactional flag set --- .../hop/pipeline/engines/local/LocalPipelineEngine.java | 6 ++++++ .../hop/workflow/engines/local/LocalWorkflowEngine.java | 6 ++++++ 2 files changed, 12 insertions(+) diff --git a/engine/src/main/java/org/apache/hop/pipeline/engines/local/LocalPipelineEngine.java b/engine/src/main/java/org/apache/hop/pipeline/engines/local/LocalPipelineEngine.java index 273a3d5bca9..771f57f8b51 100644 --- a/engine/src/main/java/org/apache/hop/pipeline/engines/local/LocalPipelineEngine.java +++ b/engine/src/main/java/org/apache/hop/pipeline/engines/local/LocalPipelineEngine.java @@ -149,6 +149,9 @@ public void prepareExecution() throws HopException { "Error committing database connection " + database.getDatabaseMeta().getName(), e); + } finally { + // Close connection always! + database.closeConnectionOnly(); } } else { try { @@ -164,6 +167,9 @@ public void prepareExecution() throws HopException { "Error rolling back database connection " + database.getDatabaseMeta().getName(), e); + } finally { + // Close connection always! + database.closeConnectionOnly(); } } } diff --git a/engine/src/main/java/org/apache/hop/workflow/engines/local/LocalWorkflowEngine.java b/engine/src/main/java/org/apache/hop/workflow/engines/local/LocalWorkflowEngine.java index c2f93ce47b9..c18736d84c0 100644 --- a/engine/src/main/java/org/apache/hop/workflow/engines/local/LocalWorkflowEngine.java +++ b/engine/src/main/java/org/apache/hop/workflow/engines/local/LocalWorkflowEngine.java @@ -134,6 +134,9 @@ public Result startExecution() { + database.getDatabaseMeta().getName(), e); result.setNrErrors(result.getNrErrors() + 1); + } finally { + // Close connection always! + database.closeConnectionOnly(); } } else { // Error? Rollback! @@ -153,6 +156,9 @@ public Result startExecution() { + database.getDatabaseMeta().getName(), e); result.setNrErrors(result.getNrErrors() + 1); + } finally { + // Close connection always! + database.closeConnectionOnly(); } } } From 7db4d7dbe581b990ecbab2bb6432a34d7fda8d42 Mon Sep 17 00:00:00 2001 From: "sergio.ramazzina" Date: Mon, 10 Jan 2022 22:09:43 +0100 Subject: [PATCH 2/3] HOP-3673 Minor improvements --- .../engines/local/LocalPipelineEngine.java | 44 +++++++++++++++++-- .../engines/local/LocalWorkflowEngine.java | 44 +++++++++++++++++-- 2 files changed, 80 insertions(+), 8 deletions(-) diff --git a/engine/src/main/java/org/apache/hop/pipeline/engines/local/LocalPipelineEngine.java b/engine/src/main/java/org/apache/hop/pipeline/engines/local/LocalPipelineEngine.java index 771f57f8b51..71afe1c73c2 100644 --- a/engine/src/main/java/org/apache/hop/pipeline/engines/local/LocalPipelineEngine.java +++ b/engine/src/main/java/org/apache/hop/pipeline/engines/local/LocalPipelineEngine.java @@ -150,8 +150,26 @@ public void prepareExecution() throws HopException { + database.getDatabaseMeta().getName(), e); } finally { - // Close connection always! - database.closeConnectionOnly(); + // Always close connection! + try { + database.closeConnectionOnly(); + pipeline + .getLogChannel() + .logDebug( + "Database connection '" + + database.getDatabaseMeta().getName() + + "' closed successfully!"); + } catch (HopDatabaseException ignoredKde) { + // The only exception thrown from closeConnectionOnly() + // cannot do anything about this but log it + pipeline + .getLogChannel() + .logError( + "Error disconnecting from database - closeConnectionOnly failed:" + + Const.CR + + ignoredKde.getMessage()); + pipeline.getLogChannel().logError(Const.getStackTracker(ignoredKde)); + } } } else { try { @@ -168,8 +186,26 @@ public void prepareExecution() throws HopException { + database.getDatabaseMeta().getName(), e); } finally { - // Close connection always! - database.closeConnectionOnly(); + // Always close connection! + try { + database.closeConnectionOnly(); + pipeline + .getLogChannel() + .logDebug( + "Database connection '" + + database.getDatabaseMeta().getName() + + "' closed successfully!"); + } catch (HopDatabaseException ignoredKde) { + // The only exception thrown from closeConnectionOnly() + // cannot do anything about this but log it + pipeline + .getLogChannel() + .logError( + "Error disconnecting from database - closeConnectionOnly failed:" + + Const.CR + + ignoredKde.getMessage()); + pipeline.getLogChannel().logError(Const.getStackTracker(ignoredKde)); + } } } } diff --git a/engine/src/main/java/org/apache/hop/workflow/engines/local/LocalWorkflowEngine.java b/engine/src/main/java/org/apache/hop/workflow/engines/local/LocalWorkflowEngine.java index c18736d84c0..987d6f64efb 100644 --- a/engine/src/main/java/org/apache/hop/workflow/engines/local/LocalWorkflowEngine.java +++ b/engine/src/main/java/org/apache/hop/workflow/engines/local/LocalWorkflowEngine.java @@ -135,8 +135,26 @@ public Result startExecution() { e); result.setNrErrors(result.getNrErrors() + 1); } finally { - // Close connection always! - database.closeConnectionOnly(); + // Always close connection! + try { + database.closeConnectionOnly(); + workflow + .getLogChannel() + .logDebug( + "Database connection '" + + database.getDatabaseMeta().getName() + + "' closed successfully!"); + } catch (HopDatabaseException ignoredKde) { + // The only exception thrown from closeConnectionOnly() + // cannot do anything about this but log it + workflow + .getLogChannel() + .logError( + "Error disconnecting from database - closeConnectionOnly failed:" + + Const.CR + + ignoredKde.getMessage()); + workflow.getLogChannel().logError(Const.getStackTracker(ignoredKde)); + } } } else { // Error? Rollback! @@ -157,8 +175,26 @@ public Result startExecution() { e); result.setNrErrors(result.getNrErrors() + 1); } finally { - // Close connection always! - database.closeConnectionOnly(); + // Always close connection! + try { + database.closeConnectionOnly(); + workflow + .getLogChannel() + .logDebug( + "Database connection '" + + database.getDatabaseMeta().getName() + + "' closed successfully!"); + } catch (HopDatabaseException ignoredKde) { + // The only exception thrown from closeConnectionOnly() + // cannot do anything about this but log it + workflow + .getLogChannel() + .logError( + "Error disconnecting from database - closeConnectionOnly failed:" + + Const.CR + + ignoredKde.getMessage()); + workflow.getLogChannel().logError(Const.getStackTracker(ignoredKde)); + } } } } From 20442451c65d550f7c74df6d99296567dc8c46fb Mon Sep 17 00:00:00 2001 From: "sergio.ramazzina" Date: Tue, 11 Jan 2022 17:19:05 +0100 Subject: [PATCH 3/3] HOP-3673 Code refactoring --- .../apache/hop/core/database/Database.java | 7 +- .../engines/local/LocalPipelineEngine.java | 97 +++++++---------- .../engines/local/LocalWorkflowEngine.java | 103 +++++++----------- 3 files changed, 82 insertions(+), 125 deletions(-) diff --git a/core/src/main/java/org/apache/hop/core/database/Database.java b/core/src/main/java/org/apache/hop/core/database/Database.java index 34243b20535..aae56c23f71 100644 --- a/core/src/main/java/org/apache/hop/core/database/Database.java +++ b/core/src/main/java/org/apache/hop/core/database/Database.java @@ -512,13 +512,12 @@ public synchronized void disconnect() { try { closeConnectionOnly(); } catch ( - HopDatabaseException ignoredKde) { // The only exception thrown from closeConnectionOnly() - // cannot do anything about this but log it + HopDatabaseException hde) { log.logError( "Error disconnecting from database - closeConnectionOnly failed:" + Const.CR - + ignoredKde.getMessage()); - log.logError(Const.getStackTracker(ignoredKde)); + + hde.getMessage()); + log.logError(Const.getStackTracker(hde)); } } } diff --git a/engine/src/main/java/org/apache/hop/pipeline/engines/local/LocalPipelineEngine.java b/engine/src/main/java/org/apache/hop/pipeline/engines/local/LocalPipelineEngine.java index 71afe1c73c2..fc15848d330 100644 --- a/engine/src/main/java/org/apache/hop/pipeline/engines/local/LocalPipelineEngine.java +++ b/engine/src/main/java/org/apache/hop/pipeline/engines/local/LocalPipelineEngine.java @@ -135,77 +135,56 @@ public void prepareExecution() throws HopException { for (Database database : databases) { // All fine? Commit! // - if (result.getResult() && !result.isStopped() && result.getNrErrors() == 0) { - try { - database.commit(true); - pipeline - .getLogChannel() - .logBasic( - "All transactions of database connection '" - + database.getDatabaseMeta().getName() - + "' were committed at the end of the pipeline!"); - } catch (HopDatabaseException e) { - throw new HopException( - "Error committing database connection " - + database.getDatabaseMeta().getName(), - e); - } finally { - // Always close connection! + try { + if (result.getResult() && !result.isStopped() && result.getNrErrors() == 0) { try { - database.closeConnectionOnly(); + database.commit(true); pipeline .getLogChannel() - .logDebug( - "Database connection '" + .logBasic( + "All transactions of database connection '" + database.getDatabaseMeta().getName() - + "' closed successfully!"); - } catch (HopDatabaseException ignoredKde) { - // The only exception thrown from closeConnectionOnly() - // cannot do anything about this but log it + + "' were committed at the end of the pipeline!"); + } catch (HopDatabaseException e) { + throw new HopException( + "Error committing database connection " + + database.getDatabaseMeta().getName(), + e); + } + } else { + try { + database.rollback(true); pipeline .getLogChannel() - .logError( - "Error disconnecting from database - closeConnectionOnly failed:" - + Const.CR - + ignoredKde.getMessage()); - pipeline.getLogChannel().logError(Const.getStackTracker(ignoredKde)); + .logBasic( + "All transactions of database connection '" + + database.getDatabaseMeta().getName() + + "' were rolled back at the end of the pipeline!"); + } catch (HopDatabaseException e) { + throw new HopException( + "Error rolling back database connection " + + database.getDatabaseMeta().getName(), + e); } } - } else { + } finally { + // Always close connection! try { - database.rollback(true); + database.closeConnectionOnly(); pipeline .getLogChannel() - .logBasic( - "All transactions of database connection '" + .logDebug( + "Database connection '" + database.getDatabaseMeta().getName() - + "' were rolled back at the end of the pipeline!"); - } catch (HopDatabaseException e) { - throw new HopException( - "Error rolling back database connection " - + database.getDatabaseMeta().getName(), - e); - } finally { - // Always close connection! - try { - database.closeConnectionOnly(); - pipeline - .getLogChannel() - .logDebug( - "Database connection '" - + database.getDatabaseMeta().getName() - + "' closed successfully!"); - } catch (HopDatabaseException ignoredKde) { - // The only exception thrown from closeConnectionOnly() - // cannot do anything about this but log it - pipeline - .getLogChannel() - .logError( - "Error disconnecting from database - closeConnectionOnly failed:" - + Const.CR - + ignoredKde.getMessage()); - pipeline.getLogChannel().logError(Const.getStackTracker(ignoredKde)); - } + + "' closed successfully!"); + } catch (HopDatabaseException hde) { + pipeline + .getLogChannel() + .logError( + "Error disconnecting from database - closeConnectionOnly failed:" + + Const.CR + + hde.getMessage()); + pipeline.getLogChannel().logError(Const.getStackTracker(hde)); } } } diff --git a/engine/src/main/java/org/apache/hop/workflow/engines/local/LocalWorkflowEngine.java b/engine/src/main/java/org/apache/hop/workflow/engines/local/LocalWorkflowEngine.java index 987d6f64efb..8f95b2fc0d7 100644 --- a/engine/src/main/java/org/apache/hop/workflow/engines/local/LocalWorkflowEngine.java +++ b/engine/src/main/java/org/apache/hop/workflow/engines/local/LocalWorkflowEngine.java @@ -117,85 +117,64 @@ public Result startExecution() { for (Database database : databases) { // All fine? Commit! // - if (result.getResult() && !result.isStopped() && result.getNrErrors() == 0) { - try { - database.commit(true); - workflow - .getLogChannel() - .logBasic( - "All transactions of database connection '" - + database.getDatabaseMeta().getName() - + "' were committed at the end of the workflow!"); - } catch (HopDatabaseException e) { - workflow - .getLogChannel() - .logError( - "Error committing database connection " - + database.getDatabaseMeta().getName(), - e); - result.setNrErrors(result.getNrErrors() + 1); - } finally { - // Always close connection! + try { + if (result.getResult() && !result.isStopped() && result.getNrErrors() == 0) { try { - database.closeConnectionOnly(); + database.commit(true); workflow .getLogChannel() - .logDebug( - "Database connection '" + .logBasic( + "All transactions of database connection '" + database.getDatabaseMeta().getName() - + "' closed successfully!"); - } catch (HopDatabaseException ignoredKde) { - // The only exception thrown from closeConnectionOnly() - // cannot do anything about this but log it + + "' were committed at the end of the workflow!"); + } catch (HopDatabaseException e) { workflow .getLogChannel() .logError( - "Error disconnecting from database - closeConnectionOnly failed:" - + Const.CR - + ignoredKde.getMessage()); - workflow.getLogChannel().logError(Const.getStackTracker(ignoredKde)); + "Error committing database connection " + + database.getDatabaseMeta().getName(), + e); + result.setNrErrors(result.getNrErrors() + 1); } - } - } else { - // Error? Rollback! - try { - database.rollback(true); - workflow - .getLogChannel() - .logBasic( - "All transactions of database connection '" - + database.getDatabaseMeta().getName() - + "' were rolled back at the end of the workflow!"); - } catch (HopDatabaseException e) { - workflow - .getLogChannel() - .logError( - "Error rolling back database connection " - + database.getDatabaseMeta().getName(), - e); - result.setNrErrors(result.getNrErrors() + 1); - } finally { - // Always close connection! + } else { + // Error? Rollback! try { - database.closeConnectionOnly(); + database.rollback(true); workflow .getLogChannel() - .logDebug( - "Database connection '" + .logBasic( + "All transactions of database connection '" + database.getDatabaseMeta().getName() - + "' closed successfully!"); - } catch (HopDatabaseException ignoredKde) { - // The only exception thrown from closeConnectionOnly() - // cannot do anything about this but log it + + "' were rolled back at the end of the workflow!"); + } catch (HopDatabaseException e) { workflow .getLogChannel() .logError( - "Error disconnecting from database - closeConnectionOnly failed:" - + Const.CR - + ignoredKde.getMessage()); - workflow.getLogChannel().logError(Const.getStackTracker(ignoredKde)); + "Error rolling back database connection " + + database.getDatabaseMeta().getName(), + e); + result.setNrErrors(result.getNrErrors() + 1); } } + } finally { + // Always close connection! + try { + database.closeConnectionOnly(); + workflow + .getLogChannel() + .logDebug( + "Database connection '" + + database.getDatabaseMeta().getName() + + "' closed successfully!"); + } catch (HopDatabaseException hde) { + workflow + .getLogChannel() + .logError( + "Error disconnecting from database - closeConnectionOnly failed:" + + Const.CR + + hde.getMessage()); + workflow.getLogChannel().logError(Const.getStackTracker(hde)); + } } } });