#47587 hard coded limit of 64 masters in agreement and changelog code
Closed: wontfix None Opened 10 years ago by rmeggins.

repl5.h:

define MAX_NUM_OF_MASTERS 64

repl5_agmt.c:
typedef struct repl5agmt {
...
struct changecounter changecounters[MAX_NUM_OF_MASTERS]; / changes sent/skipped since server start up */
...
}

cl5_cache.c:
struct clc_buffer {
...
struct csn_seq_ctrl_block *buf_cscbs [MAX_NUM_OF_MASTERS];
}


For me the changes are good. ack

Two minor remarks:
in agmt_maxcsn_to_smod, the 'smod' is initialized before testing if there is agreements.
If there is no agreement, it returns 1 so the 'smod->mod' may leak

in agmt_set_maxcsn, ra->maxcsn=NULL is suppressed. If for some reason we do not find maxcsn in the db ruv then ra->maxcsn will be unchanged (no call to slapi_ch_free_string(&ra->maxcsn)). Is that what you expect ?

Replying to [comment:7 tbordaz]:

For me the changes are good. ack

Two minor remarks:
in agmt_maxcsn_to_smod, the 'smod' is initialized before testing if there is agreements.
If there is no agreement, it returns 1 so the 'smod->mod' may leak

I made that change so it wouldn't leak. It is only called from replica_write_ruv(), where it will always free the smod that was initialized in agmt_maxcsn_to_smod().

I will add a comment to the function stating that the smod always needs to be freed - even on error.

in agmt_set_maxcsn, ra->maxcsn=NULL is suppressed. If for some reason we do not find maxcsn in the db ruv then ra->maxcsn will be unchanged (no call to slapi_ch_free_string(&ra->maxcsn)). Is that what you expect ?

Yes, we don't want to remove the old maxcsn value unless there is a new one to replace it with. In fact this only called when the agmt is started, so usually it is NULL to being with. Unless the agmt has been stopped and restarted, etc. Either way, we still don't want to remove the old value unless its being replaced.

I would rather have the memory leak fixes be in a separate commit, in case we need to cherry pick separately.

Replying to [comment:9 rmeggins]:

I would rather have the memory leak fixes be in a separate commit, in case we need to cherry pick separately.

New patch is attached. I'll do another commit for the ticket that actually introduced the memory leaks.

git merge ticket47587
Updating 3155bbb..bae797c
Fast-forward
ldap/servers/plugins/replication/cl5_clcache.c | 22 +++++++++++++++++-----
ldap/servers/plugins/replication/repl5.h | 4 ++--
ldap/servers/plugins/replication/repl5_agmt.c | 18 ++++++++++++++----
3 files changed, 33 insertions(+), 11 deletions(-)

git push origin master
Counting objects: 17, done.
Delta compression using up to 4 threads.
Compressing objects: 100% (9/9), done.
Writing objects: 100% (9/9), 1.50 KiB, done.
Total 9 (delta 7), reused 0 (delta 0)
To ssh://git.fedorahosted.org/git/389/ds.git
3155bbb..bae797c master -> master

commit bae797c
Author: Mark Reynolds mreynolds@redhat.com
Date: Thu Dec 5 11:58:56 2013 -0500

1.3.2

b3b9c45..df5a061 389-ds-base-1.3.2 -> 389-ds-base-1.3.2

1.3.1

f00321f..457cd16 389-ds-base-1.3.1 -> 389-ds-base-1.3.1

1.3.0

b4e73f3..63e919f 389-ds-base-1.3.0 -> 389-ds-base-1.3.0

1.2.11

766e747..b5622fb 389-ds-base-1.2.11 -> 389-ds-base-1.2.11

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

7 years ago

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/924

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.

Metadata Update from @spichugi:
- Issue close_status updated to: wontfix (was: Fixed)

3 years ago

Login to comment on this ticket.

Metadata