Bug 228 - 'drop node' in a try block
Summary: 'drop node' in a try block
Status: RESOLVED FIXED
Alias: None
Product: Slony-I
Classification: Unclassified
Component: slonik (show other bugs)
Version: devel
Hardware: PC Linux
: low enhancement
Assignee: Steve Singer
URL:
Depends on:
Blocks:
 
Reported: 2011-08-04 13:28 UTC by Steve Singer
Modified: 2011-08-15 05:57 UTC (History)
1 user (show)

See Also:

Attachments
patch for a possible fix (5.25 KB, patch)
2011-08-04 13:41 UTC, Steve Singer 		Details
Add an attachment (proposed patch, testcase, etc.)

Note You need to log in before you can comment on or make changes to this bug.
Description Steve Singer 2011-08-04 13:28:26 UTC 
In doing some testing against the 2.1.0 beta I tried executing some 
slonik code like

try {
   drop node(id=3, event node=1);
}
on error {
   echo "node already gone";
}
store node(id=3, event node=1);

A script like this will fail in the current 2.1.0 betas and this 
shouldn't surprise anyone who has read about the new features in 2.1.0

A drop node requires that the cluster be somewhat caught up (at least to 
the extent that any events from the drop'd node that have been confirmed 
elsewhere are confirmed everywhere).  This means that drop node has an 
implicit 'wait for event' before it.   However you can't have 'wait for 
events' inside a try block.
Comment 1 Steve Singer 2011-08-04 13:41:21 UTC 
Created attachment 117 [details]
patch for a possible fix

This patch allows the first useful command in a try block to be a waiting operation.  It will rollback the transaction and create a new one after the wait.
Comment 2 Christopher Browne 2011-08-08 12:39:36 UTC 
Another alternative would be for conditionals to be handled in an "abort if {conditions} fashion", as described in bug #173.

I think you've observed that this isn't necessarily a reason to "abort," so perhaps that should change to "conditional execution."
Comment 3 Steve Singer 2011-08-10 12:51:37 UTC 
Please review
Comment 4 Jan Wieck 2011-08-12 14:07:31 UTC 
Looks good to me. Please apply.