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.

revision
0001-Ticket-47819-Improve-tombstone-purging-performance.patch

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

Fix coverity issue(resource leak)
0001-Ticket-47819-Fix-memory-leak.patch

fe59338..ca3d08a master -> master
commit ca3d08a

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

Ticket has been cloned to Bugzilla: https://bugzilla.redhat.com/show_bug.cgi?id=1356258

389-ds-base is moving from Pagure to Github. This means that new issues and pull requests
will be accepted only in 389-ds-base's github repository.

This issue has been cloned to Github and is available here:
- https://github.com/389ds/389-ds-base/issues/1150

If you want to receive further updates on the issue, please navigate to the github issue
and click on subscribe button.

Thank you for understanding. We apologize for all inconvenience.