Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Changes caused by ALTER DATABASE command are not replicated #8008

Open
pavel-zotov opened this issue Feb 16, 2024 · 19 comments
Open

Changes caused by ALTER DATABASE command are not replicated #8008

pavel-zotov opened this issue Feb 16, 2024 · 19 comments

Comments

@pavel-zotov
Copy link

Most (or all ?) changes caused by the 'ALTER DATABASE' command are not replicated.
Below are steps to reproduce on Windows (checked on 6.0.0.264).

Stop FB server.
Create folder %FB_HOME%\examples\empbuild\qa_replication\
Create two databases and prepare them for work in replication. This is example of replication.conf:

database
{
  verbose_logging = true
}

database = $(dir_sampleDb)/qa_replication/db_main.fdb
{
    journal_directory = "$(dir_sampleDb)/qa_replication/repl_journal"
    journal_archive_directory = "$(dir_sampleDb)/qa_replication/repl_archive"
    journal_archive_command = "copy $(pathname) $(archivepathname)"
    journal_archive_timeout = 10
}

database = $(dir_sampleDb)/qa_replication/db_repl.fdb
{
    journal_source_directory = "$(dir_sampleDb)/qa_replication/repl_archive"
    apply_idle_timeout = 3
    apply_error_timeout = 7
}

Copy all files from %FB_HOME%\examples\prebuilt\plugins\ to %FB_HOME%\plugins\
Add following lines in %FB_HOME%\plugins\fbSampleKeyHolder.conf

Auto = true
KeyRed=111

Add parameter: KeyHolderPlugin = fbSampleKeyHolder in firebird.conf

Start FB server.

Ensure that there are no files c:\temp\tmp_result_1.log and c:\temp\tmp_result_2.log, and run following script:

set bail on;
set list on;
set count on;

connect 'localhost:db_main_alias' user sysdba password 'masterkey';


recreate table test1(id int primary key, v varchar(10));

alter database set default sql security definer;
alter database set default character set win1251;

recreate table test2(id int primary key, v varchar(10));

recreate table test3(id int primary key, v varchar(10));

alter database exclude table test3 from publication;
alter database encrypt with "fbSampleDbCrypt" key "Red";
commit;

-------------------------------------------------------------------------------------
shell timeout 5; -- keep connection active and let ecnryption thread complete its job
-------------------------------------------------------------------------------------

shell del c:\temp\tmp_result_1.log;
out c:\temp\tmp_result_1.log;

set echo on;
select
    m.mon$database_name as db_name
    ,r.rdb$sql_security as db_default_security
    ,r.rdb$character_set_name as db_default_cset
    ,m.mon$crypt_state as db_crypt_state
    ,p.rdb$table_name as pubicated_table
from rdb$database r 
cross join mon$database m
left join rdb$publication_tables p on 1=1 
;

show table test1;

show table test2;

out;
set echo off;
commit;

------------------------------------------------------------
shell timeout 15; -- let all replication segments be applied
------------------------------------------------------------

connect 'localhost:db_repl_alias' user sysdba password 'masterkey';

shell del c:\temp\tmp_result_2.log;
out c:\temp\tmp_result_2.log;

set echo on;
select
    m.mon$database_name as db_name
    ,r.rdb$sql_security as db_default_security
    ,r.rdb$character_set_name as db_default_cset
    ,m.mon$crypt_state as db_crypt_state
    ,p.rdb$table_name as pubicated_table
from rdb$database r 
cross join mon$database m
left join rdb$publication_tables p on 1=1 
;

show table test1;

show table test2;

out;
set echo off;

Compare logs: fc /w /n C:\temp\tmp_result_1.log C:\temp\tmp_result_2.log
Output will be:

***** C:\TEMP\tmp_result_1.log
   10:  ;
   12:  DB_NAME                         C:\FB\60SS\examples\empbuild\qa_replication\db_main.fdb
   13:  DB_DEFAULT_SECURITY             <true>
   14:  DB_DEFAULT_CSET                 WIN1251                                                                                        
   17:  DB_CRYPT_STATE                  1
   18:  PUBICATED_TABLE                 TEST1                                                                                          
   22:  DB_NAME                         C:\FB\60SS\examples\empbuild\qa_replication\db_main.fdb
   23:  DB_DEFAULT_SECURITY             <true>
   24:  DB_DEFAULT_CSET                 WIN1251                                                                                        
   27:  DB_CRYPT_STATE                  1
   28:  PUBICATED_TABLE                 TEST2                                                                                          
   33:  Records affected: 2
   35:  show table test1;
***** C:\TEMP\TMP_RESULT_2.LOG
   10:  ;
   12:  DB_NAME                         C:\FB\60SS\examples\empbuild\qa_replication\db_repl.fdb
   13:  DB_DEFAULT_SECURITY             <null> -- <<<<<<<<<<<<<<<<<< [ 1 ]
   14:  DB_DEFAULT_CSET                 NONE   -- <<<<<<<<<<<<<<<<<< [ 2 ]
   17:  DB_CRYPT_STATE                  0      -- <<<<<<<<<<<<<<<<<< [ 3 ]
   18:  PUBICATED_TABLE                 <null> -- <<<<<<<<<<<<<<<<<< [ 4 ]
   21:  Records affected: 1
   23:  show table test1;
*****

***** C:\TEMP\tmp_result_1.log
   39:    Primary key (ID)
   40:  Publications: RDB$DEFAULT (Enabled)
   42:  show table test2;
***** C:\TEMP\TMP_RESULT_2.LOG
   27:    Primary key (ID)
   29:  show table test2;
*****

***** C:\TEMP\tmp_result_1.log
   43:  ID                              INTEGER Not Null 
   44:  V                               VARCHAR(10) CHARACTER SET WIN1251 Nullable 
   45:  CONSTRAINT INTEG_4:
***** C:\TEMP\TMP_RESULT_2.LOG
   30:  ID                              INTEGER Not Null 
   31:  V                               VARCHAR(10) CHARACTER SET NONE Nullable -- <<<<<<< [ ! ]
   32:  CONSTRAINT INTEG_4:
*****

***** C:\TEMP\tmp_result_1.log
   46:    Primary key (ID)
   47:  Publications: RDB$DEFAULT (Enabled)
   49:  out;
***** C:\TEMP\TMP_RESULT_2.LOG
   33:    Primary key (ID)
   35:  out;
*****

diff-main-repl

PS.
Same for LINGER. Although this parameter does not affect anything, but it would be more convenient for DBAs that its value will be also replicated - just in case when replica becomes master because of hardware crash etc.

@dyemanov dyemanov self-assigned this Feb 16, 2024
@dyemanov
Copy link
Member

dyemanov commented Feb 16, 2024

Most of the ALTER DATABASE sub-commands are intentionally not replicated, but some others should be but currently not.

@pavel-zotov
Copy link
Author

OK, but what about these:

   14:  DB_DEFAULT_CSET                 NONE   -- <<<<<<<<<<<<<<<<<< [ 2 ]
   17:  DB_CRYPT_STATE                  0      -- <<<<<<<<<<<<<<<<<< [ 3 ]
   18:  PUBICATED_TABLE                 <null> -- <<<<<<<<<<<<<<<<<< [ 4 ]

-- ?
Shouldn't all these parameters be replicated ?

@dyemanov
Copy link
Member

Now I seem to recall the problem. Multiple options can be specified inside one ALTER DATABASE statement. And some of these options should not be replicated, but some others - should be. But currently it's impossible to split the command and replicate only one its part.

ALTER DATABASE BEGIN BACKUP DROP LINGER SET DEFAULT CHARACTER SET win1251 DISABLE PUBLICATION;

What should we do with this? ;-)

@dyemanov
Copy link
Member

dyemanov commented Feb 16, 2024

This is why it's currently prohibited as a whole and replica must be re-initialized after "sensible" ALTER DATABASE commands.

@pavel-zotov
Copy link
Author

This is from doc:

alter {database | schema}
    { <add_sec_clause> [<add_sec_clausee> ...] }
  | {add difference file 'diff_file' | drop difference fi
  | {{begin | end} backup}
  | {set default character set charset}
  | {set default sql security {definer | invoker}}
  | {set linger to linger_duration| drop linger}
  | {encrypt with plugin_name[key key_name] | decrypt}
  | {enable | disable} publication
  | include {table <table_list> | all} to publication
  | exclude {table <table_list> | all} from publication

IMO, only one option ('begin backup') must not be replicated ('add difference file' is atavism and not used).
So, when 'alter database' is issued in mixed ('compound') form - why this single option can not be either prohibited or just be skipped from replication ?

@mrotteveel
Copy link
Member

mrotteveel commented Feb 16, 2024 via email

@mrotteveel mrotteveel changed the title Changes caused by ALTE DATABASE command are not replicated Changes caused by ALTER DATABASE command are not replicated Feb 16, 2024
@dyemanov
Copy link
Member

I'd say these should be replicated:
SET DEFAULT CHARACTER SET
SET DEFAULT SQL SECURITY

maybe also:
[INCLUDE TO | EXCLUDE FROM] PUBLICATION
[ENABLE | DISABLE] PUBLICATION

not so sure about:
SET LINGER

the rest is physical database management and IMO should not be replicated.

@pavel-zotov
Copy link
Author

What do you mean with "atavism"? Do you mean legacy/deprecated?

I made the mistake of confusing it with 'add_sec_clause'.
Anyway, it will be good if all options related to backup or adding secondary file will be forced to use separately.
Their usage together with other options (DROP LINGER, SET DEFAULT CHARACTER SET, etc) must be prohibited.

@pavel-zotov
Copy link
Author

maybe also: [INCLUDE TO | EXCLUDE FROM] PUBLICATION ; [ENABLE | DISABLE] PUBLICATION

IMO, this will be very useful for DBA when he changes replica DB to master.
And also - If replica is 'intermediate' node in case of cascade replication: master -> replica1 -> replica2 (but i did not verified whether this possible at all :))

@pavel-zotov
Copy link
Author

PS. and what about encryption ?

@dyemanov
Copy link
Member

Encryption IMHO should not be replicated, because replica security implications may be different from the master. DBA will need to re-encrypt the db after failover.

@dyemanov
Copy link
Member

dyemanov commented Feb 16, 2024

maybe also: [INCLUDE TO | EXCLUDE FROM] PUBLICATION ; [ENABLE | DISABLE] PUBLICATION

IMO, this will be very useful for DBA when he changes replica DB to master. And also - If replica is 'intermediate' node in case of cascade replication: master -> replica1 -> replica2 (but i did not verified whether this possible at all :))

If cascade replication is enabled and replica is read-write, disabling replication on the primary node will force it being disabled on the replica too, but there maybe replica-only modifications that still must be replicated further. So generally I don't think that [ENABLE | DISABLE] PUBLICATION is safe to replicate. I don't mind replicating INCLUDE/EXCLUDE though.

@dyemanov
Copy link
Member

Anyway, it will be good if all options related to backup or adding secondary file will be forced to use separately. Their usage together with other options (DROP LINGER, SET DEFAULT CHARACTER SET, etc) must be prohibited.

Please create a separate ticket for this.

@sim1984
Copy link

sim1984 commented Feb 16, 2024

Only

alter {database | schema}
     {set default character set charset}
   | {set default sql security {definer | invoker}}

can be safely replicated.

For include | exclude There are doubts it can be deliberately changed on the replica side during cascade replication. It seems to me that they need a separate setting.

@AlexPeshkoff
Copy link
Member

AlexPeshkoff commented Feb 16, 2024 via email

@dyemanov
Copy link
Member

What about replicating ALTER DB partially (not all options) - there is simple solution with rebuilding SQL statement based on replicated clauses.

Yes, I've already created the patch ;-) Just need to figure out what options it should handle.

@aafemt
Copy link
Contributor

aafemt commented Feb 16, 2024

What should we do with this? ;-)

Make ALTER DATABASE to ignore some clauses if database is in replica mode IMHO.

@dyemanov
Copy link
Member

What should we do with this? ;-)

Make ALTER DATABASE to ignore some clauses if database is in replica mode IMHO.

Maybe this gonna be more changes than the master-side solution mentioned above (too many clauses to add a check for) but I suppose it should look prettier. I will give it a try.

@aafemt
Copy link
Contributor

aafemt commented Feb 16, 2024

I forgot to add "and issue a warning" to "ignore". DBA would be very confused if they try to execute ALTER DATABASE on replica and it silently do nothing.

Also perhaps master should not try to omit replication of query that contain only would-be-ignored clauses because replication plugin or target server can have different opinion about it.

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

No branches or pull requests

6 participants