#47819 RFE - improve tombstone purging performance
Closed: Fixed None Opened 5 years ago by mreynolds.

Tombstone purging/reaping can be very expensive, especially if the number of tombstones exceeds the idlistscanlimit. The tombstone purging thread issues internal subtree searches for "(objectlclass=nsTombstone)".

Perhaps set the objectclass index scan limit for "objectclass: nstombstone" to a very large value(or unlimited).


I strongly do not want to have backups/db files/other binary files in the git repository. Isn't there some way that the test can create these files at test time?

Replying to [comment:4 rmeggins]:

I strongly do not want to have backups/db files/other binary files in the git repository. Isn't there some way that the test can create these files at test time?

Not in the current version of lib389. I need a backup or database from an older version of 389 to test the fixup task. For now I can remove the test for the fixup task(backup files) until lib389 can handle multi-version servers.

Replying to [comment:5 mreynolds]:

Replying to [comment:4 rmeggins]:

I strongly do not want to have backups/db files/other binary files in the git repository. Isn't there some way that the test can create these files at test time?

Not in the current version of lib389. I need a backup or database from an older version of 389 to test the fixup task.

How much older?

For now I can remove the test for the fixup task(backup files) until lib389 can handle multi-version servers.

Replying to [comment:6 rmeggins]:

Replying to [comment:5 mreynolds]:

Replying to [comment:4 rmeggins]:

I strongly do not want to have backups/db files/other binary files in the git repository. Isn't there some way that the test can create these files at test time?

Not in the current version of lib389. I need a backup or database from an older version of 389 to test the fixup task.

How much older?

Pre-patch. My fix automatically starts maintaining the new index, so I need to create a backup from a server that does not have my patch applied. Then restore a db backup into a server that does have my patch applied. So it's kind of simulating an upgrade. Then run the fix up task.

Replying to [comment:7 mreynolds]:

Replying to [comment:6 rmeggins]:

Replying to [comment:5 mreynolds]:

Replying to [comment:4 rmeggins]:

I strongly do not want to have backups/db files/other binary files in the git repository. Isn't there some way that the test can create these files at test time?

Not in the current version of lib389. I need a backup or database from an older version of 389 to test the fixup task.

How much older?

Pre-patch. My fix automatically starts maintaining the new index, so I need to create a backup from a server that does not have my patch applied. Then restore a db backup into a server that does have my patch applied. So it's kind of simulating an upgrade. Then run the fix up task.

Ok. Can the test do this?
1) create a db with some tombstones
2) db2ldif -r to export those tombstones
3) edit the ldif to remove the nstombstonecsn attributes
4) remove the index configuration for the nstombstonecsn index
5) import the ldif
6) db2bak
the backup should then be in the "pre-upgrade" state

Then you can reconfigure the server to use the nstombstonecsn index and do the tests with the older database.

{{{
- if(!op->o_reverse_search_state){
+ if(!op->o_reverse_search_state && sr->sr_candidates){
}}}

This looks unrelated - is this another bug?

{{{

define TASK_SYSCONFIG_RELOAD_DN "cn=sysconfig reload,cn=tasks,cn=config"

}}}

This patch adds the sysconfig reload task DN?

Replying to [comment:8 rmeggins]:

Replying to [comment:7 mreynolds]:

Replying to [comment:6 rmeggins]:

Replying to [comment:5 mreynolds]:

Replying to [comment:4 rmeggins]:

I strongly do not want to have backups/db files/other binary files in the git repository. Isn't there some way that the test can create these files at test time?

Not in the current version of lib389. I need a backup or database from an older version of 389 to test the fixup task.

How much older?

Pre-patch. My fix automatically starts maintaining the new index, so I need to create a backup from a server that does not have my patch applied. Then restore a db backup into a server that does have my patch applied. So it's kind of simulating an upgrade. Then run the fix up task.

Ok. Can the test do this?
1) create a db with some tombstones
2) db2ldif -r to export those tombstones
3) edit the ldif to remove the nstombstonecsn attributes
4) remove the index configuration for the nstombstonecsn index
5) import the ldif
6) db2bak
the backup should then be in the "pre-upgrade" state

Then you can reconfigure the server to use the nstombstonecsn index and do the tests with the older database.

I'll give this a shot, but it is hardcoded to add nstombstoneCSN during imports.

{{{
- if(!op->o_reverse_search_state){
+ if(!op->o_reverse_search_state && sr->sr_candidates){
}}}

This looks unrelated - is this another bug?

This fixed a crash I ran into when creating/testing the fixup task.

{{{

define TASK_SYSCONFIG_RELOAD_DN "cn=sysconfig reload,cn=tasks,cn=config"

}}}

This patch adds the sysconfig reload task DN?

Oh yeah, unrelated I suppose, I was just adding it for completeness in task.c.

Ok. Can the test do this?
1) create a db with some tombstones
2) db2ldif -r to export those tombstones
3) edit the ldif to remove the nstombstonecsn attributes
4) remove the index configuration for the nstombstonecsn index
5) import the ldif
6) db2bak
the backup should then be in the "pre-upgrade" state

Then you can reconfigure the server to use the nstombstonecsn index and do the tests with the older database.

I'll give this a shot, but it is hardcoded to add nstombstoneCSN during imports.

Yeah the attribute is still added to the entry during the import(import_producer).

I could add a hidden option to the fixup task to strip nsTombstoneCSN instead of adding it. I'm not seeing another option without making larger changes to the patch just so we can test the fixup task.

Replying to [comment:10 mreynolds]:

Ok. Can the test do this?
1) create a db with some tombstones
2) db2ldif -r to export those tombstones
3) edit the ldif to remove the nstombstonecsn attributes
4) remove the index configuration for the nstombstonecsn index
5) import the ldif
6) db2bak
the backup should then be in the "pre-upgrade" state

Then you can reconfigure the server to use the nstombstonecsn index and do the tests with the older database.

I'll give this a shot, but it is hardcoded to add nstombstoneCSN during imports.

Yeah the attribute is still added to the entry during the import(import_producer).

I could add a hidden option to the fixup task to strip nsTombstoneCSN instead of adding it.

Ok. That sounds like the best option for testing.

I'm not seeing another option without making larger changes to the patch just so we can test the fixup task.

Replying to [comment:11 rmeggins]:

Replying to [comment:10 mreynolds]:

Ok. Can the test do this?
1) create a db with some tombstones
2) db2ldif -r to export those tombstones
3) edit the ldif to remove the nstombstonecsn attributes
4) remove the index configuration for the nstombstonecsn index
5) import the ldif
6) db2bak
the backup should then be in the "pre-upgrade" state

Then you can reconfigure the server to use the nstombstonecsn index and do the tests with the older database.

I'll give this a shot, but it is hardcoded to add nstombstoneCSN during imports.

Yeah the attribute is still added to the entry during the import(import_producer).

I could add a hidden option to the fixup task to strip nsTombstoneCSN instead of adding it.

Ok. That sounds like the best option for testing.

New patch is attached.

The latest patch still has the sysconfig reload and the sr->sr_candidates patches. Those should really be in separate tickets & patches.

git merge ticket47818
Updating e6cee31..0dfe006
Fast-forward
Makefile.am | 1 +
Makefile.in | 1 +
aclocal.m4 | 15 +++
config.guess | 151 ++++++++++++---------------
config.sub | 30 +++---
dirsrvtests/data/README | 11 ++
dirsrvtests/tickets/ticket47819_test.py | 345 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
dirsrvtests/tmp/README | 10 ++
ldap/admin/src/scripts/50nstombstonecsn.ldif | 7 ++
ldap/ldif/template-dse.ldif.in | 7 ++
ldap/schema/01core389.ldif | 6 +-
ldap/servers/plugins/replication/repl5.h | 4 +-
ldap/servers/plugins/replication/repl5_replica.c | 67 +++++++++++-
ldap/servers/plugins/replication/repl5_replica_config.c | 60 ++++++++---
ldap/servers/plugins/replication/repl_globals.c | 1 +
ldap/servers/plugins/replication/urp_tombstone.c | 2 +-
ldap/servers/slapd/back-ldbm/import-threads.c | 39 +++++++
ldap/servers/slapd/back-ldbm/index.c | 13 +++
ldap/servers/slapd/back-ldbm/ldbm_add.c | 59 ++++++++++-
ldap/servers/slapd/back-ldbm/ldbm_delete.c | 79 ++++++++++++--
ldap/servers/slapd/back-ldbm/ldbm_modify.c | 15 ++-
ldap/servers/slapd/back-ldbm/ldbm_modrdn.c | 1 +
ldap/servers/slapd/back-ldbm/ldif2ldbm.c | 63 +++++++++++
ldap/servers/slapd/entrywsi.c | 23 ++++
ldap/servers/slapd/mapping_tree.c | 11 +-
ldap/servers/slapd/slapi-plugin.h | 2 +
ldap/servers/slapd/slapi-private.h | 4 +-
ldap/servers/slapd/task.c | 352 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++----
28 files changed, 1215 insertions(+), 164 deletions(-)
create mode 100644 dirsrvtests/data/README
create mode 100644 dirsrvtests/tickets/ticket47819_test.py
create mode 100644 dirsrvtests/tmp/README
create mode 100644 ldap/admin/src/scripts/50nstombstonecsn.ldif

git push origin master
e6cee31..0dfe006 master -> master

commit 0dfe006
Author: Mark Reynolds mreynolds@redhat.com
Date: Mon Jul 14 12:04:26 2014 -0400

Although replica->precise_purging has a Slapi_Counter type:
{{{
repl5_replica.c: Slapi_Counter precise_purging; / Enable precise tombstone purging */

repl5_replica.c: slapi_counter_set_value(r->precise_purging, 1);
}}}
a counter is not generated for precise_purging...
$ egrep slapi_counter_new *.c
repl5_agmt.c: ra->protocol_timeout = slapi_counter_new();
repl5_replica.c: r->protocol_timeout = slapi_counter_new();
repl5_replica.c: r->backoff_min = slapi_counter_new();
repl5_replica.c: r->backoff_max = slapi_counter_new();

git patch file (master) -- adding slapi_counter_new and _destroy for replica->precise_purging
0001-Ticket-47819-RFE-improve-tombstone-purging-performan.patch

Wow, so I guess it was always evaluated as "on"? Good catch!

Thanks for reviewing the change, Mark!!

Pushed to master:
e6ba94f..fb39031 master -> master
commit fb39031

Pushed to 389-ds-base-1.3.4, as well:
7600d88..d5a84c4 389-ds-base-1.3.4 -> 389-ds-base-1.3.4
commit d5a84c4

Metadata Update from @mreynolds:
- Issue assigned to mreynolds
- Issue set to the milestone: 1.3.3 backlog

2 years ago

Login to comment on this ticket.

Metadata