Skip to content

Support for tablespaces - #8314

Open
Zhdanov0 wants to merge 162 commits into
FirebirdSQL:masterfrom
Zhdanov0:port2firebird_tablespaces2
Open

Zhdanov0 wants to merge 162 commits into
FirebirdSQL:masterfrom
Zhdanov0:port2firebird_tablespaces2

Conversation

@Zhdanov0

Copy link
Copy Markdown
Contributor

romansimakov and others added 30 commits January 9, 2021 20:56
only file deleting is left at post DFW stage.
Added missed object type of TABLESPACES.
Added security class for tablespaces which allows not to
skip check of the privileges.
Corrected page space before fetching of the root index
page.
…code as for deleting a table. NOTE: Now we fire DDL trigger for deleling tables even if they are deleting as result of DROP TABLESPACE ... INCLUDING CONTENTS
…nt of usage have the page space in the pace space manager. Also the same behaviour has been ported to the expression index handling.
The recent change added a couple of new fields into CreationIndex which were not initialized before fast loading while index moving.
…espace creation

Page space should be added into pageSpaces array after creating/opening of tablespace file to prevent other attachments from getting incomplete page space.
…ME> INCLUDING CONTENTS. Deleting from tablespaces cache is implemented.
…e rollback work correctly. The file will be created at DFW stage
… A relation may already be deleted by dfw_delete_relation in the same transaction
It allows to determine if there are attachments that use a tablespace (and have its page space opened) when another attachment needs the EX lock on that tablespace (for example, to execute ALTER TABLESPACE).
…rash while trying to write dirty pages of closed tablespace to disk
Comment thread doc/README.tablespaces
ALTER INDEX ...
[SET TABLESPACE [TO] {<TS NAME> | PRIMARY}]

The index data will be moved to the specified tablespace or to the main database.

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

May be to add about necessary locks for the index movement.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Locks are documented now

Comment thread doc/README.tablespaces

ALTER TABLE <TABLE NAME> SET TABLESPACE [TO] {<TS NAME> | PRIMARY}

The table data will be moved to the specified tablespace or to the main database.

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

May be add about necessary locks for the table's data movement

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Locks are documented now

Comment thread doc/README.tablespaces Outdated
If you do not specify the above options, when restoring a database that has tablespaces,
an error about the inability to determine the path to restore tablespaces will occur.

3. Show

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

is it a command of isql?

Comment thread doc/README.tablespaces Outdated
- allocate necessary number of pointer pages by extents.
- allocate the rest of pointer pages by pages.
- walking through PPs allocate DPs by pages or extents.
- fix every PP by correcting DP numbers and ppg_next pointer and build a map

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

still allocate new data pages on demand for data pages with "orphan" flags which are not in the PP.

Comment thread src/jrd/jrd.cpp Outdated
return status->getState() & IStatus::STATE_ERRORS ? true : false;
}

static bool drop_files(ObjectsArray<PathName>& tsFiles)

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just "files" is better than "tsFiles".

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed and slightly refactored to simplify the code even more.

Comment thread src/jrd/met.epp Outdated
}
}

ULONG pageSpaceId = DB_PAGE_SPACE;

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The same code as above. New function or lambda?

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Removed the redundancy

Comment thread src/jrd/Monitoring.cpp
MetaName name;

if (pageSpaceId == 0)
name = TEMP_TABLESPACE_NAME;

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe to move the logic under Tablespace::lookup?

Comment thread src/jrd/Tablespace.h Outdated
Tablespace* get(ULONG id);
Tablespace* get(const MetaName& name);

void store(Tablespace* tabpeSpace);

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

typo

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed

Comment thread src/jrd/Tablespace.h Outdated
Firebird::Mutex m_mutex;
};

explicit Tablespace(MemoryPool& pool, ULONG id = DB_PAGE_SPACE)

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There is no backward compatibility, so maybe better not to use default value for id?

@dyemanov dyemanov Sep 17, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Replaced with an explicit ID passed (only a single place is affected)

Comment thread src/dsql/DdlNodes.epp Outdated

if (tableSpace.hasData() && applyTablespacesDdl(tdbb))
{
if ((tableSpace != PRIMARY_TABLESPACE_NAME) &&

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Here and later Is this check is necessary? AFAIU checkObjectExist takes into account special case that tablespace is PRIMARY.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Simplified and made consistent with the handling of other object types.

@dyemanov

Copy link
Copy Markdown
Member

I've addressed most of the review comments, a few are still in the pipeline and will be finished this week.

@dyemanov

Copy link
Copy Markdown
Member

For gstat, we have also added a -ts switch that allows to analyze only particular tablespace. It will be added as a separate PR (unless you think it's not needed for Firebird).

Comment thread doc/README.tablespaces

4. Replication
There is an apply_tablespaces_ddl parameter for replication, it may be configured for the replica side.
If this parameter is disabled, CREATE/ALTER/DROP TSABLESPACE statements will not be ignored on the replica and

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

will not be ignored ?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Tablespaces [CORE688]

9 participants