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 "Knut Anders Hatlen (JIRA)" <ji...@apache.org> on 2007/10/08 16:06:50 UTC
[jira] Created: (DERBY-3116) totalSpace not properly initialized in
AllocPage
totalSpace not properly initialized in AllocPage
------------------------------------------------
Key: DERBY-3116
URL: https://issues.apache.org/jira/browse/DERBY-3116
Project: Derby
Issue Type: Bug
Components: Store
Affects Versions: 10.4.0.0
Reporter: Knut Anders Hatlen
Priority: Minor
There are some problems with the initialization of totalSpace in AllocPage. It is initialized in StoredPage.initSpace() which is again called from StoredPage.usePageBuffer(), and it is set to the value returned from AllocPage.getMaxFreeSpace(). The problems are:
1) The calculation in getMaxFreeSpace() uses borrowedSpace, but when createIdentity() is called on an AllocPage, borrowedSpace has not been initialized when getMaxFreeSpace() is called and the calculated size is wrong.
2) When a page object is reused, usePageBuffer() is only called if a new byte array must be allocated (because the new page has a different size than the old page). This means that the totalSize field gets the same value as in the old page if their sizes are equal, which is not necessarily correct.
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Commented: (DERBY-3116) totalSpace not properly initialized
in AllocPage
Posted by "Knut Anders Hatlen (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/DERBY-3116?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#action_12534795 ]
Knut Anders Hatlen commented on DERBY-3116:
-------------------------------------------
Thanks for looking at the patch, Jørgen!
> it seems a little overkill to always reinitiate the byte[] in StoredPage.
Note that setPageBuffer() doesn't reallocate the byte array, it only sets the reference to the byte array and refreshes some of the instance variables. I think the cost of these operations is negligible compared to the cost of evicting the old page and initializing the new one.
> Could you achieve the same thing by moving the initSpace call from StoredPage#usePageBuffer to StoredPage#createPage?
That might be possible, but then I think we would also have to move it into StoredPage.initFromData() as the same problem may occur if you read an alloc page from disk instead of creating a new one. It also seems like initSpace() depends on other variables initialized in usePageBuffer() (slotEntrySize in particular) so we'd probably end up with moving most of the code in usePageBuffer() into initSpace() anyway.
> Another bug? In StoredPage#initSpace, slotEntrySize is used when setting maxFieldSize. However, initSpace is called before slotEntrySize has been updated in usePageBuffer.
This is supposed to be fixed in the current trunk (see DERBY-3099). But thanks for verifiying that it in fact is a bug! :)
> Not related to your patch: There is a problem with the Javadoc in AllocPage (unclosed paragraph "<p"). Fixing that would show more javadoc in the html files.
Good catch! I fixed the tag and checked it in. Now I see one more line in the javadoc, but still there's much of it that doesn't show up. Perhaps there's a problem with the custom javadoc tags (format_id, purpose, etc)?
> totalSpace not properly initialized in AllocPage
> ------------------------------------------------
>
> Key: DERBY-3116
> URL: https://issues.apache.org/jira/browse/DERBY-3116
> Project: Derby
> Issue Type: Bug
> Components: Store
> Affects Versions: 10.4.0.0
> Reporter: Knut Anders Hatlen
> Priority: Minor
> Attachments: d3116-1.diff
>
>
> There are some problems with the initialization of totalSpace in AllocPage. It is initialized in StoredPage.initSpace() which is again called from StoredPage.usePageBuffer(), and it is set to the value returned from AllocPage.getMaxFreeSpace(). The problems are:
> 1) The calculation in getMaxFreeSpace() uses borrowedSpace, but when createIdentity() is called on an AllocPage, borrowedSpace has not been initialized when getMaxFreeSpace() is called and the calculated size is wrong.
> 2) When a page object is reused, usePageBuffer() is only called if a new byte array must be allocated (because the new page has a different size than the old page). This means that the totalSize field gets the same value as in the old page if their sizes are equal, which is not necessarily correct.
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Commented: (DERBY-3116) totalSpace not properly initialized
in AllocPage
Posted by "Knut Anders Hatlen (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/DERBY-3116?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12585511#action_12585511 ]
Knut Anders Hatlen commented on DERBY-3116:
-------------------------------------------
Committed to trunk with revision 644698.
> totalSpace not properly initialized in AllocPage
> ------------------------------------------------
>
> Key: DERBY-3116
> URL: https://issues.apache.org/jira/browse/DERBY-3116
> Project: Derby
> Issue Type: Bug
> Components: Store
> Affects Versions: 10.4.0.0
> Reporter: Knut Anders Hatlen
> Assignee: Knut Anders Hatlen
> Priority: Minor
> Attachments: d3116-1.diff, d3116-2.diff
>
>
> There are some problems with the initialization of totalSpace in AllocPage. It is initialized in StoredPage.initSpace() which is again called from StoredPage.usePageBuffer(), and it is set to the value returned from AllocPage.getMaxFreeSpace(). The problems are:
> 1) The calculation in getMaxFreeSpace() uses borrowedSpace, but when createIdentity() is called on an AllocPage, borrowedSpace has not been initialized when getMaxFreeSpace() is called and the calculated size is wrong.
> 2) When a page object is reused, usePageBuffer() is only called if a new byte array must be allocated (because the new page has a different size than the old page). This means that the totalSize field gets the same value as in the old page if their sizes are equal, which is not necessarily correct.
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Commented: (DERBY-3116) totalSpace not properly initialized
in AllocPage
Posted by "Knut Anders Hatlen (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/DERBY-3116?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#action_12533187 ]
Knut Anders Hatlen commented on DERBY-3116:
-------------------------------------------
suites.All and derbyall ran cleanly with the d3116-1 patch.
> totalSpace not properly initialized in AllocPage
> ------------------------------------------------
>
> Key: DERBY-3116
> URL: https://issues.apache.org/jira/browse/DERBY-3116
> Project: Derby
> Issue Type: Bug
> Components: Store
> Affects Versions: 10.4.0.0
> Reporter: Knut Anders Hatlen
> Priority: Minor
> Attachments: d3116-1.diff
>
>
> There are some problems with the initialization of totalSpace in AllocPage. It is initialized in StoredPage.initSpace() which is again called from StoredPage.usePageBuffer(), and it is set to the value returned from AllocPage.getMaxFreeSpace(). The problems are:
> 1) The calculation in getMaxFreeSpace() uses borrowedSpace, but when createIdentity() is called on an AllocPage, borrowedSpace has not been initialized when getMaxFreeSpace() is called and the calculated size is wrong.
> 2) When a page object is reused, usePageBuffer() is only called if a new byte array must be allocated (because the new page has a different size than the old page). This means that the totalSize field gets the same value as in the old page if their sizes are equal, which is not necessarily correct.
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Commented: (DERBY-3116) totalSpace not properly initialized
in AllocPage
Posted by "Jørgen Løland (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/DERBY-3116?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#action_12534775 ]
Jørgen Løland commented on DERBY-3116:
--------------------------------------
Hi Knut Anders.
Patch d3116-1 looks correct to me, but it seems a little overkill to always reinitiate the byte[] in StoredPage. Could you achieve the same thing by moving the initSpace call from StoredPage#usePageBuffer to StoredPage#createPage? Alternatively make initSpace protected and call StoredPage#initSpace from CachedPage#setPageArray when the page is reused?
Another bug? In StoredPage#initSpace, slotEntrySize is used when setting maxFieldSize. However, initSpace is called before slotEntrySize has been updated in usePageBuffer.
Not related to your patch: There is a problem with the Javadoc in AllocPage (unclosed paragraph "<p"). Fixing that would show more javadoc in the html files.
> totalSpace not properly initialized in AllocPage
> ------------------------------------------------
>
> Key: DERBY-3116
> URL: https://issues.apache.org/jira/browse/DERBY-3116
> Project: Derby
> Issue Type: Bug
> Components: Store
> Affects Versions: 10.4.0.0
> Reporter: Knut Anders Hatlen
> Priority: Minor
> Attachments: d3116-1.diff
>
>
> There are some problems with the initialization of totalSpace in AllocPage. It is initialized in StoredPage.initSpace() which is again called from StoredPage.usePageBuffer(), and it is set to the value returned from AllocPage.getMaxFreeSpace(). The problems are:
> 1) The calculation in getMaxFreeSpace() uses borrowedSpace, but when createIdentity() is called on an AllocPage, borrowedSpace has not been initialized when getMaxFreeSpace() is called and the calculated size is wrong.
> 2) When a page object is reused, usePageBuffer() is only called if a new byte array must be allocated (because the new page has a different size than the old page). This means that the totalSize field gets the same value as in the old page if their sizes are equal, which is not necessarily correct.
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Updated: (DERBY-3116) totalSpace not properly initialized in
AllocPage
Posted by "Knut Anders Hatlen (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/DERBY-3116?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]
Knut Anders Hatlen updated DERBY-3116:
--------------------------------------
Attachment: d3116-1.diff
I haven't run any tests on it yet, but this patch seems to make totalSpace have the correct value for all pages created in unit/T_RawStoreFactory.unit (verified by printing and comparing totalSpace to getMaxFreeSpace()). Will start a full run of regression tests and report back. I'll also see if I can add some asserts (since I know of no other way to expose the bug).
What this patch does, is:
- in AllocPage.createPage(), set borrowedSpace before super.createPage() is called so that getMaxFreeSpace() returns the correct value when totalSpace is initialized
- in CachedPage.setPageArray(), call usePageBuffer() also when the old buffer is reused. This ensures that totalSpace is recalculated when a page object is reused.
> totalSpace not properly initialized in AllocPage
> ------------------------------------------------
>
> Key: DERBY-3116
> URL: https://issues.apache.org/jira/browse/DERBY-3116
> Project: Derby
> Issue Type: Bug
> Components: Store
> Affects Versions: 10.4.0.0
> Reporter: Knut Anders Hatlen
> Priority: Minor
> Attachments: d3116-1.diff
>
>
> There are some problems with the initialization of totalSpace in AllocPage. It is initialized in StoredPage.initSpace() which is again called from StoredPage.usePageBuffer(), and it is set to the value returned from AllocPage.getMaxFreeSpace(). The problems are:
> 1) The calculation in getMaxFreeSpace() uses borrowedSpace, but when createIdentity() is called on an AllocPage, borrowedSpace has not been initialized when getMaxFreeSpace() is called and the calculated size is wrong.
> 2) When a page object is reused, usePageBuffer() is only called if a new byte array must be allocated (because the new page has a different size than the old page). This means that the totalSize field gets the same value as in the old page if their sizes are equal, which is not necessarily correct.
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Updated: (DERBY-3116) totalSpace not properly initialized in
AllocPage
Posted by "Knut Anders Hatlen (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/DERBY-3116?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]
Knut Anders Hatlen updated DERBY-3116:
--------------------------------------
Attachment: d3116-2.diff
Uploading a new patch which resolves a conflict with a recent commit. The new patch also contains an assert which exposes the bug in the lack of a test for it.
- Without the fixes, the assert will cause database creation to fail.
- With the fix in AllocPage.createPage(), unit/T_RawStoreFactory.unit will fail when an AllocPage is evicted from the page cache to make room for an AllocPage with a different borrowedSpace value.
- With the fix in CachedPage.setPageArray(), unit/T_RawStoreFactory.unit passes.
I'm re-running derbyall and suites.All now. Since there were no more comments on the previous patch, I intend to commit the updated patch to trunk and 10.4 if all the tests pass.
> totalSpace not properly initialized in AllocPage
> ------------------------------------------------
>
> Key: DERBY-3116
> URL: https://issues.apache.org/jira/browse/DERBY-3116
> Project: Derby
> Issue Type: Bug
> Components: Store
> Affects Versions: 10.4.0.0
> Reporter: Knut Anders Hatlen
> Assignee: Knut Anders Hatlen
> Priority: Minor
> Attachments: d3116-1.diff, d3116-2.diff
>
>
> There are some problems with the initialization of totalSpace in AllocPage. It is initialized in StoredPage.initSpace() which is again called from StoredPage.usePageBuffer(), and it is set to the value returned from AllocPage.getMaxFreeSpace(). The problems are:
> 1) The calculation in getMaxFreeSpace() uses borrowedSpace, but when createIdentity() is called on an AllocPage, borrowedSpace has not been initialized when getMaxFreeSpace() is called and the calculated size is wrong.
> 2) When a page object is reused, usePageBuffer() is only called if a new byte array must be allocated (because the new page has a different size than the old page). This means that the totalSize field gets the same value as in the old page if their sizes are equal, which is not necessarily correct.
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Assigned: (DERBY-3116) totalSpace not properly initialized
in AllocPage
Posted by "Knut Anders Hatlen (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/DERBY-3116?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]
Knut Anders Hatlen reassigned DERBY-3116:
-----------------------------------------
Assignee: Knut Anders Hatlen
> totalSpace not properly initialized in AllocPage
> ------------------------------------------------
>
> Key: DERBY-3116
> URL: https://issues.apache.org/jira/browse/DERBY-3116
> Project: Derby
> Issue Type: Bug
> Components: Store
> Affects Versions: 10.4.0.0
> Reporter: Knut Anders Hatlen
> Assignee: Knut Anders Hatlen
> Priority: Minor
> Attachments: d3116-1.diff
>
>
> There are some problems with the initialization of totalSpace in AllocPage. It is initialized in StoredPage.initSpace() which is again called from StoredPage.usePageBuffer(), and it is set to the value returned from AllocPage.getMaxFreeSpace(). The problems are:
> 1) The calculation in getMaxFreeSpace() uses borrowedSpace, but when createIdentity() is called on an AllocPage, borrowedSpace has not been initialized when getMaxFreeSpace() is called and the calculated size is wrong.
> 2) When a page object is reused, usePageBuffer() is only called if a new byte array must be allocated (because the new page has a different size than the old page). This means that the totalSize field gets the same value as in the old page if their sizes are equal, which is not necessarily correct.
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Resolved: (DERBY-3116) totalSpace not properly initialized
in AllocPage
Posted by "Knut Anders Hatlen (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/DERBY-3116?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]
Knut Anders Hatlen resolved DERBY-3116.
---------------------------------------
Resolution: Fixed
Fix Version/s: 10.5.0.0
10.4.1.0
Committed to 10.4 with revision 644967.
> totalSpace not properly initialized in AllocPage
> ------------------------------------------------
>
> Key: DERBY-3116
> URL: https://issues.apache.org/jira/browse/DERBY-3116
> Project: Derby
> Issue Type: Bug
> Components: Store
> Affects Versions: 10.4.0.0
> Reporter: Knut Anders Hatlen
> Assignee: Knut Anders Hatlen
> Priority: Minor
> Fix For: 10.4.1.0, 10.5.0.0
>
> Attachments: d3116-1.diff, d3116-2.diff
>
>
> There are some problems with the initialization of totalSpace in AllocPage. It is initialized in StoredPage.initSpace() which is again called from StoredPage.usePageBuffer(), and it is set to the value returned from AllocPage.getMaxFreeSpace(). The problems are:
> 1) The calculation in getMaxFreeSpace() uses borrowedSpace, but when createIdentity() is called on an AllocPage, borrowedSpace has not been initialized when getMaxFreeSpace() is called and the calculated size is wrong.
> 2) When a page object is reused, usePageBuffer() is only called if a new byte array must be allocated (because the new page has a different size than the old page). This means that the totalSize field gets the same value as in the old page if their sizes are equal, which is not necessarily correct.
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.