You are viewing a plain text version of this content. The canonical link for it is here.
Posted to derby-dev@db.apache.org by "Kristian Waagan (JIRA)" <ji...@apache.org> on 2010/11/25 11:51:13 UTC
[jira] Created: (DERBY-4918) Minor refactoring of SPSDescriptor
Minor refactoring of SPSDescriptor
----------------------------------
Key: DERBY-4918
URL: https://issues.apache.org/jira/browse/DERBY-4918
Project: Derby
Issue Type: Improvement
Components: SQL
Affects Versions: 10.8.0.0
Reporter: Kristian Waagan
Assignee: Kristian Waagan
Priority: Minor
The class SPSDescriptor is kind of hard to understand, and doesn't quite follow the pattern used by other tuple descriptors. Parts of the code don't agree with the documentation (i.e. SPS_TYPE_TRIGGER marked as not implemented)
This issue tracks work to make the class easier to understand and to modify.
The critical part that may need to be changed is the use of synchronized (this). The problem is that database locks are obtained within the critical regions, and in some special cases this causes deadlocks. I'm not yet certain this can be fixed at this level (only), but I'll continue investigation.
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Closed: (DERBY-4918) Minor refactoring of SPSDescriptor
Posted by "Kristian Waagan (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/DERBY-4918?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]
Kristian Waagan closed DERBY-4918.
----------------------------------
Resolution: Fixed
Closing issue.
> Minor refactoring of SPSDescriptor
> ----------------------------------
>
> Key: DERBY-4918
> URL: https://issues.apache.org/jira/browse/DERBY-4918
> Project: Derby
> Issue Type: Improvement
> Components: SQL
> Affects Versions: 10.8.0.0
> Reporter: Kristian Waagan
> Assignee: Kristian Waagan
> Priority: Minor
> Fix For: 10.8.0.0
>
> Attachments: derby-4918-1a-misc.diff, derby-4918-1a-misc.stat, derby-4918-2a-remove_setUUID.diff, derby-4918-2b-remove_setUUID.diff
>
>
> The class SPSDescriptor is kind of hard to understand, and doesn't quite follow the pattern used by other tuple descriptors. Parts of the code don't agree with the documentation (i.e. SPS_TYPE_TRIGGER marked as not implemented)
> This issue tracks work to make the class easier to understand and to modify.
> The critical part that may need to be changed is the use of synchronized (this). The problem is that database locks are obtained within the critical regions, and in some special cases this causes deadlocks. I'm not yet certain this can be fixed at this level (only), but I'll continue investigation.
--
This message is automatically generated by JIRA.
-
For more information on JIRA, see: http://www.atlassian.com/software/jira
[jira] Updated: (DERBY-4918) Minor refactoring of SPSDescriptor
Posted by "Kristian Waagan (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/DERBY-4918?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]
Kristian Waagan updated DERBY-4918:
-----------------------------------
Issue & fix info: (was: [Patch Available])
> Minor refactoring of SPSDescriptor
> ----------------------------------
>
> Key: DERBY-4918
> URL: https://issues.apache.org/jira/browse/DERBY-4918
> Project: Derby
> Issue Type: Improvement
> Components: SQL
> Affects Versions: 10.8.0.0
> Reporter: Kristian Waagan
> Assignee: Kristian Waagan
> Priority: Minor
> Fix For: 10.8.0.0
>
> Attachments: derby-4918-1a-misc.diff, derby-4918-1a-misc.stat, derby-4918-2a-remove_setUUID.diff, derby-4918-2b-remove_setUUID.diff
>
>
> The class SPSDescriptor is kind of hard to understand, and doesn't quite follow the pattern used by other tuple descriptors. Parts of the code don't agree with the documentation (i.e. SPS_TYPE_TRIGGER marked as not implemented)
> This issue tracks work to make the class easier to understand and to modify.
> The critical part that may need to be changed is the use of synchronized (this). The problem is that database locks are obtained within the critical regions, and in some special cases this causes deadlocks. I'm not yet certain this can be fixed at this level (only), but I'll continue investigation.
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Updated: (DERBY-4918) Minor refactoring of SPSDescriptor
Posted by "Kristian Waagan (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/DERBY-4918?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]
Kristian Waagan updated DERBY-4918:
-----------------------------------
Issue & fix info: [Patch Available]
> Minor refactoring of SPSDescriptor
> ----------------------------------
>
> Key: DERBY-4918
> URL: https://issues.apache.org/jira/browse/DERBY-4918
> Project: Derby
> Issue Type: Improvement
> Components: SQL
> Affects Versions: 10.8.0.0
> Reporter: Kristian Waagan
> Assignee: Kristian Waagan
> Priority: Minor
> Attachments: derby-4918-1a-misc.diff, derby-4918-1a-misc.stat
>
>
> The class SPSDescriptor is kind of hard to understand, and doesn't quite follow the pattern used by other tuple descriptors. Parts of the code don't agree with the documentation (i.e. SPS_TYPE_TRIGGER marked as not implemented)
> This issue tracks work to make the class easier to understand and to modify.
> The critical part that may need to be changed is the use of synchronized (this). The problem is that database locks are obtained within the critical regions, and in some special cases this causes deadlocks. I'm not yet certain this can be fixed at this level (only), but I'll continue investigation.
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Updated: (DERBY-4918) Minor refactoring of SPSDescriptor
Posted by "Kristian Waagan (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/DERBY-4918?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]
Kristian Waagan updated DERBY-4918:
-----------------------------------
Attachment: derby-4918-2a-remove_setUUID.diff
Attaching patch 2a, which removes the method setUUID.
I made the getUUID-method synchronized as part of patch 1a. Turns out this caused a Java deadlock in one test when running with the automatic index statistics update feature.
Investigation revealed that this method can actually be removed, which makes the uuid variable final. This allows for keeping getUUID unsynchronized.
I did add code to fail if null is passed in as null in the constructor, where a NullPointerException will be thrown.
suites.All passed, will run derbyall too.
Patch ready for review.
> Minor refactoring of SPSDescriptor
> ----------------------------------
>
> Key: DERBY-4918
> URL: https://issues.apache.org/jira/browse/DERBY-4918
> Project: Derby
> Issue Type: Improvement
> Components: SQL
> Affects Versions: 10.8.0.0
> Reporter: Kristian Waagan
> Assignee: Kristian Waagan
> Priority: Minor
> Fix For: 10.8.0.0
>
> Attachments: derby-4918-1a-misc.diff, derby-4918-1a-misc.stat, derby-4918-2a-remove_setUUID.diff
>
>
> The class SPSDescriptor is kind of hard to understand, and doesn't quite follow the pattern used by other tuple descriptors. Parts of the code don't agree with the documentation (i.e. SPS_TYPE_TRIGGER marked as not implemented)
> This issue tracks work to make the class easier to understand and to modify.
> The critical part that may need to be changed is the use of synchronized (this). The problem is that database locks are obtained within the critical regions, and in some special cases this causes deadlocks. I'm not yet certain this can be fixed at this level (only), but I'll continue investigation.
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Updated: (DERBY-4918) Minor refactoring of SPSDescriptor
Posted by "Kristian Waagan (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/DERBY-4918?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]
Kristian Waagan updated DERBY-4918:
-----------------------------------
Issue & fix info: [Patch Available]
> Minor refactoring of SPSDescriptor
> ----------------------------------
>
> Key: DERBY-4918
> URL: https://issues.apache.org/jira/browse/DERBY-4918
> Project: Derby
> Issue Type: Improvement
> Components: SQL
> Affects Versions: 10.8.0.0
> Reporter: Kristian Waagan
> Assignee: Kristian Waagan
> Priority: Minor
> Fix For: 10.8.0.0
>
> Attachments: derby-4918-1a-misc.diff, derby-4918-1a-misc.stat, derby-4918-2a-remove_setUUID.diff
>
>
> The class SPSDescriptor is kind of hard to understand, and doesn't quite follow the pattern used by other tuple descriptors. Parts of the code don't agree with the documentation (i.e. SPS_TYPE_TRIGGER marked as not implemented)
> This issue tracks work to make the class easier to understand and to modify.
> The critical part that may need to be changed is the use of synchronized (this). The problem is that database locks are obtained within the critical regions, and in some special cases this causes deadlocks. I'm not yet certain this can be fixed at this level (only), but I'll continue investigation.
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Updated: (DERBY-4918) Minor refactoring of SPSDescriptor
Posted by "Kristian Waagan (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/DERBY-4918?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]
Kristian Waagan updated DERBY-4918:
-----------------------------------
Attachment: derby-4918-1a-misc.stat
Attaching patch 1a.
Regression tests passed. Ready for review.
> Minor refactoring of SPSDescriptor
> ----------------------------------
>
> Key: DERBY-4918
> URL: https://issues.apache.org/jira/browse/DERBY-4918
> Project: Derby
> Issue Type: Improvement
> Components: SQL
> Affects Versions: 10.8.0.0
> Reporter: Kristian Waagan
> Assignee: Kristian Waagan
> Priority: Minor
> Attachments: derby-4918-1a-misc.diff, derby-4918-1a-misc.stat
>
>
> The class SPSDescriptor is kind of hard to understand, and doesn't quite follow the pattern used by other tuple descriptors. Parts of the code don't agree with the documentation (i.e. SPS_TYPE_TRIGGER marked as not implemented)
> This issue tracks work to make the class easier to understand and to modify.
> The critical part that may need to be changed is the use of synchronized (this). The problem is that database locks are obtained within the critical regions, and in some special cases this causes deadlocks. I'm not yet certain this can be fixed at this level (only), but I'll continue investigation.
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Issue Comment Edited: (DERBY-4918) Minor refactoring of
SPSDescriptor
Posted by "Knut Anders Hatlen (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/DERBY-4918?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12966513#action_12966513 ]
Knut Anders Hatlen edited comment on DERBY-4918 at 12/3/10 7:46 AM:
--------------------------------------------------------------------
Patch 2a looks like a nice cleanup. +1
One small nit is that some people (for instance these guys: http://pmd.sourceforge.net/rules/strictexception.html#AvoidThrowingNullPointerException ) claim that throwing a NullPointerException is confusing and recommend using IllegalArgumentException instead. Not that it's supposed to ever happen...
[comment edited: added a space so that JIRA doesn't garble the URL]
was (Author: knutanders):
Patch 2a looks like a nice cleanup. +1
One small nit is that some people (for instance these guys: http://pmd.sourceforge.net/rules/strictexception.html#AvoidThrowingNullPointerException) claim that throwing a NullPointerException is confusing and recommend using IllegalArgumentException instead. Not that it's supposed to ever happen...
> Minor refactoring of SPSDescriptor
> ----------------------------------
>
> Key: DERBY-4918
> URL: https://issues.apache.org/jira/browse/DERBY-4918
> Project: Derby
> Issue Type: Improvement
> Components: SQL
> Affects Versions: 10.8.0.0
> Reporter: Kristian Waagan
> Assignee: Kristian Waagan
> Priority: Minor
> Fix For: 10.8.0.0
>
> Attachments: derby-4918-1a-misc.diff, derby-4918-1a-misc.stat, derby-4918-2a-remove_setUUID.diff
>
>
> The class SPSDescriptor is kind of hard to understand, and doesn't quite follow the pattern used by other tuple descriptors. Parts of the code don't agree with the documentation (i.e. SPS_TYPE_TRIGGER marked as not implemented)
> This issue tracks work to make the class easier to understand and to modify.
> The critical part that may need to be changed is the use of synchronized (this). The problem is that database locks are obtained within the critical regions, and in some special cases this causes deadlocks. I'm not yet certain this can be fixed at this level (only), but I'll continue investigation.
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Commented: (DERBY-4918) Minor refactoring of SPSDescriptor
Posted by "Kristian Waagan (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/DERBY-4918?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12969389#action_12969389 ]
Kristian Waagan commented on DERBY-4918:
----------------------------------------
The deadlock I saw was actually a problem with BasicDependencyManager, not SPSDescriptor (see DERBY-4928).
This may mean that there is nothing more to do for this issue, I'll keep the issue open for a while to see if anything more pops up.
> Minor refactoring of SPSDescriptor
> ----------------------------------
>
> Key: DERBY-4918
> URL: https://issues.apache.org/jira/browse/DERBY-4918
> Project: Derby
> Issue Type: Improvement
> Components: SQL
> Affects Versions: 10.8.0.0
> Reporter: Kristian Waagan
> Assignee: Kristian Waagan
> Priority: Minor
> Fix For: 10.8.0.0
>
> Attachments: derby-4918-1a-misc.diff, derby-4918-1a-misc.stat, derby-4918-2a-remove_setUUID.diff, derby-4918-2b-remove_setUUID.diff
>
>
> The class SPSDescriptor is kind of hard to understand, and doesn't quite follow the pattern used by other tuple descriptors. Parts of the code don't agree with the documentation (i.e. SPS_TYPE_TRIGGER marked as not implemented)
> This issue tracks work to make the class easier to understand and to modify.
> The critical part that may need to be changed is the use of synchronized (this). The problem is that database locks are obtained within the critical regions, and in some special cases this causes deadlocks. I'm not yet certain this can be fixed at this level (only), but I'll continue investigation.
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Updated: (DERBY-4918) Minor refactoring of SPSDescriptor
Posted by "Kristian Waagan (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/DERBY-4918?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]
Kristian Waagan updated DERBY-4918:
-----------------------------------
Attachment: derby-4918-2b-remove_setUUID.diff
Thanks, Knut.
Attached patch 2b, which replaces the NPE with an IAE.
Committed to trunk with revision 1042048.
> Minor refactoring of SPSDescriptor
> ----------------------------------
>
> Key: DERBY-4918
> URL: https://issues.apache.org/jira/browse/DERBY-4918
> Project: Derby
> Issue Type: Improvement
> Components: SQL
> Affects Versions: 10.8.0.0
> Reporter: Kristian Waagan
> Assignee: Kristian Waagan
> Priority: Minor
> Fix For: 10.8.0.0
>
> Attachments: derby-4918-1a-misc.diff, derby-4918-1a-misc.stat, derby-4918-2a-remove_setUUID.diff, derby-4918-2b-remove_setUUID.diff
>
>
> The class SPSDescriptor is kind of hard to understand, and doesn't quite follow the pattern used by other tuple descriptors. Parts of the code don't agree with the documentation (i.e. SPS_TYPE_TRIGGER marked as not implemented)
> This issue tracks work to make the class easier to understand and to modify.
> The critical part that may need to be changed is the use of synchronized (this). The problem is that database locks are obtained within the critical regions, and in some special cases this causes deadlocks. I'm not yet certain this can be fixed at this level (only), but I'll continue investigation.
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Updated: (DERBY-4918) Minor refactoring of SPSDescriptor
Posted by "Kristian Waagan (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/DERBY-4918?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]
Kristian Waagan updated DERBY-4918:
-----------------------------------
Attachment: derby-4918-1a-misc.diff
> Minor refactoring of SPSDescriptor
> ----------------------------------
>
> Key: DERBY-4918
> URL: https://issues.apache.org/jira/browse/DERBY-4918
> Project: Derby
> Issue Type: Improvement
> Components: SQL
> Affects Versions: 10.8.0.0
> Reporter: Kristian Waagan
> Assignee: Kristian Waagan
> Priority: Minor
> Attachments: derby-4918-1a-misc.diff, derby-4918-1a-misc.stat
>
>
> The class SPSDescriptor is kind of hard to understand, and doesn't quite follow the pattern used by other tuple descriptors. Parts of the code don't agree with the documentation (i.e. SPS_TYPE_TRIGGER marked as not implemented)
> This issue tracks work to make the class easier to understand and to modify.
> The critical part that may need to be changed is the use of synchronized (this). The problem is that database locks are obtained within the critical regions, and in some special cases this causes deadlocks. I'm not yet certain this can be fixed at this level (only), but I'll continue investigation.
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Updated: (DERBY-4918) Minor refactoring of SPSDescriptor
Posted by "Kristian Waagan (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/DERBY-4918?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]
Kristian Waagan updated DERBY-4918:
-----------------------------------
Issue & fix info: (was: [Patch Available])
Fix Version/s: 10.8.0.0
Committed patch 1a to trunk with revision 1041192.
> Minor refactoring of SPSDescriptor
> ----------------------------------
>
> Key: DERBY-4918
> URL: https://issues.apache.org/jira/browse/DERBY-4918
> Project: Derby
> Issue Type: Improvement
> Components: SQL
> Affects Versions: 10.8.0.0
> Reporter: Kristian Waagan
> Assignee: Kristian Waagan
> Priority: Minor
> Fix For: 10.8.0.0
>
> Attachments: derby-4918-1a-misc.diff, derby-4918-1a-misc.stat
>
>
> The class SPSDescriptor is kind of hard to understand, and doesn't quite follow the pattern used by other tuple descriptors. Parts of the code don't agree with the documentation (i.e. SPS_TYPE_TRIGGER marked as not implemented)
> This issue tracks work to make the class easier to understand and to modify.
> The critical part that may need to be changed is the use of synchronized (this). The problem is that database locks are obtained within the critical regions, and in some special cases this causes deadlocks. I'm not yet certain this can be fixed at this level (only), but I'll continue investigation.
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Commented: (DERBY-4918) Minor refactoring of SPSDescriptor
Posted by "Knut Anders Hatlen (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/DERBY-4918?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12966513#action_12966513 ]
Knut Anders Hatlen commented on DERBY-4918:
-------------------------------------------
Patch 2a looks like a nice cleanup. +1
One small nit is that some people (for instance these guys: http://pmd.sourceforge.net/rules/strictexception.html#AvoidThrowingNullPointerException) claim that throwing a NullPointerException is confusing and recommend using IllegalArgumentException instead. Not that it's supposed to ever happen...
> Minor refactoring of SPSDescriptor
> ----------------------------------
>
> Key: DERBY-4918
> URL: https://issues.apache.org/jira/browse/DERBY-4918
> Project: Derby
> Issue Type: Improvement
> Components: SQL
> Affects Versions: 10.8.0.0
> Reporter: Kristian Waagan
> Assignee: Kristian Waagan
> Priority: Minor
> Fix For: 10.8.0.0
>
> Attachments: derby-4918-1a-misc.diff, derby-4918-1a-misc.stat, derby-4918-2a-remove_setUUID.diff
>
>
> The class SPSDescriptor is kind of hard to understand, and doesn't quite follow the pattern used by other tuple descriptors. Parts of the code don't agree with the documentation (i.e. SPS_TYPE_TRIGGER marked as not implemented)
> This issue tracks work to make the class easier to understand and to modify.
> The critical part that may need to be changed is the use of synchronized (this). The problem is that database locks are obtained within the critical regions, and in some special cases this causes deadlocks. I'm not yet certain this can be fixed at this level (only), but I'll continue investigation.
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.