Issue Details (XML | Word | Printable)

Key: CORE-6043
Type: Bug Bug
Status: Closed Closed
Resolution: Fixed
Priority: Minor Minor
Assignee: Vlad Khorsun
Reporter: Miroslav Djorov
Votes: 0
Watchers: 3
Operations

If you were logged in you would be able to see more operations.
Firebird Core

GTTs do not release used space

Created: 05/Apr/19 12:05 PM   Updated: 01/May/19 05:49 AM
Component/s: Engine
Affects Version/s: 2.5.7, 2.5.8
Fix Version/s: 2.5.9, 3.0.5, 4.0 Beta 2

File Attachments: 1. File GTT_TEST.sql (2 kB)


QA Status: Done successfully


 Description  « Hide
Global temporary tables created with ON COMMIT DELETE ROWS don't release used space in the temp file when COMMIT RETAINING or ROLLBACK RETAINING is used.
When inserting records in a GTT and the transaction is finished with COMMIT or ROLLBACK the space in the file is reused, but if a transaction insert records and then we call COMMIT or ROLLBACK RETAINING the temporary file fb_table_XXX keeps growing in size. In situations where there are 24/7 connections to the database the temp files are not deleted and with this constant growing they eat the disk space.
All records in the table are not visible after the COMMIT/ROLLBACK action as it supposed to be, but the space is not released.

 All   Comments   Change History   Subversion Commits      Sort Order: Ascending order - Click to sort in descending order
Miroslav Djorov added a comment - 05/Apr/19 12:08 PM
This is a test script using COMMIT RETAIN. Using ROLLBACK gives the same results

Paul Reeves added a comment - 05/Apr/19 12:41 PM
To be honest, I would expect that behaviour from COMMIT/ROLLBACK RETAINING because that is how RETAINING works.
Whether that should apply to GTT's is another matter.

Miroslav Djorov added a comment - 05/Apr/19 12:56 PM - edited
But all records are gone after RETAINING. They aren't visible any more in the transaction that inserted them. If the transaction is closed after the RETAINING, the space is still unusable. I've tried it with 2 connections to the database. One that do all the work with the GTT, and the other one just to have one so the file won't be deleted. Even after I use INSERT, COMMIT RETAINING, SELECT, COMMIT, DISCONNECT, the space is not reused from new INSERTS from new connections. And in some moment, the disk space is over and server crashes.

Vlad Khorsun added a comment - 05/Apr/19 01:18 PM
The data of GTT ON COMMIT DELETE ROWS table should be visible after commit retaining.
The actual bug is that engine keeps this data but does not make it visible after commit retaining.

The correct fix it to make records visible but it could break many existing applications that rely on
buggy behavior (GTT is empty after commit retaining).


Therefore i offer following solution:

- in v2.5 fix the disk space leakage and leave records not visible after commit retaining (i.e. clear GTT data)

- in v3 add per-database setting that will rule engine behavior:
  GTTClearOnCommitRetain = 1 will work as v2.5 above,
  GTTClearOnCommitRetain = 0 will not clear GTT data on commit retaining and will make records visible
after commit retaining.
By default GTTClearOnCommitRetain will be set to 1 to preserve backward compatibility with 2.5

- in v4 add same setting but make correct behavior (don't clear GTT data on commit retain) by default.
Also, if time permits, we will implement TRUNCATE TABLE statement (probably for GTT only for the start)

Adriano dos Santos Fernandes added a comment - 05/Apr/19 03:28 PM
Since the problem is a *clear* bug, people should not depend on it, therefor I may agree to the plan for v2.5 and v3, but for v4 I think there should not be a config allowing the bug behavior.

Sean Leyne added a comment - 05/Apr/19 03:32 PM
I agree with Adriano, it is a bug that needs to be squashed.

Dmitry Yemanov added a comment - 05/Apr/19 07:18 PM
"Commit" inside "GTTClearOnCommitRetain" looks confusing, as AFAIU the issue affects ROLLBACK RETAINING as well. Maybe something like ClearGTTAtRetaining?

Vlad Khorsun added a comment - 05/Apr/19 07:48 PM
Rollback should clear GTT in any case, isn't is ? :)

Dmitry Yemanov added a comment - 06/Apr/19 04:31 AM
Damn, surely it should ;-)

Vlad Khorsun added a comment - 09/Apr/19 04:13 PM
After more thinking on issue I'm not so sure that rollback retaining should clear GTT.
If we agree that GTT's data is part of transaction context and both commit retaining and rollback retaining should
preserve transaction context than we could accept that rollback retaining should undo actions made since
previous commit\rollback retaining only. For example:

create global temporary table gtt (id int)
  on commit delete rows;

insert into gtt values (1);
commit retain;

-- point 1
select * from gtt;

insert into gtt values (2);
rollback retain;

-- point 2
select * from gtt;


What results is expected at points (1) and (2) ?

The possible answers are:
a) current (buggy?) behavior
  no rows at both points

b) gtt data is preserved on both commit and rollback retaining
  one row at point (1), one row at point (2)

c) gtt data is preserved on commit retaining and cleared on rollback retaining
  one row at point (1), no rows at point (2)

PS Unfortunately, there is no exact definition of "retaining" in IB docs, so we must decide ourself

Vlad Khorsun added a comment - 09/Apr/19 04:27 PM
The patch against disk space leakage is committed into v2.5,
please try next snapshot build.

Adriano dos Santos Fernandes added a comment - 09/Apr/19 04:39 PM
I don't understand the (B) option. Please explain:

- What value it will see (1 or 2)?
- What you mean by "gtt data is preserved" in "rollback retaining"?

Vlad Khorsun added a comment - 09/Apr/19 04:48 PM
> - What value it will see (1 or 2)?

1 of course, second insert must be undone by rollback - there is no question

> - What you mean by "gtt data is preserved" in "rollback retaining"?

technically, rollback retaining could:
- release all pages of GTT (clear GTT data), or
- undo actions made since last retaining point only (preseve data existed before last retaining point)

Adriano dos Santos Fernandes added a comment - 09/Apr/19 04:57 PM
> - undo actions made since last retaining point only (preseve data existed before last retaining point)

So I agree with B option.

Dmitry Yemanov added a comment - 09/Apr/19 05:06 PM
So do I.