search

389ds / 389-ds-base

The enterprise-class Open Source LDAP server for Linux
https://www.port389.org/
Other
210 stars 90 forks source link

hard coded limit of 64 masters in agreement and changelog code #924

Closed 389-ds-bot closed 4 years ago

389-ds-bot commented 4 years ago

Cloned from Pagure issue: https://pagure.io/389-ds-base/issue/47587

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]; }

389-ds-bot commented 4 years ago

Comment from nkinder (@nkinder) at 2013-11-16 05:04:16

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

389-ds-bot commented 4 years ago

Comment from tbordaz (@tbordaz) at 2013-12-05 15:11:40

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 ?

389-ds-bot commented 4 years ago

Comment from mreynolds (@mreynolds389) at 2013-12-05 20:16:25

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.

389-ds-bot commented 4 years ago

Comment from rmeggins (@richm) at 2013-12-05 20:56:41

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

389-ds-bot commented 4 years ago

Comment from mreynolds (@mreynolds389) at 2013-12-05 22:59:42

revision 0001-Ticket-47587-hard-coded-limit-of-64-masters-in-agree.patch

389-ds-bot commented 4 years ago

Comment from mreynolds (@mreynolds389) at 2013-12-05 23:01:31

Replying to [comment:9 richm]:

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.

389-ds-bot commented 4 years ago

Comment from mreynolds (@mreynolds389) at 2013-12-05 23:42:28

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 bae797c94207d15025e763cfea0634f42eeb1210 Author: Mark Reynolds mreynolds389@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

389-ds-bot commented 4 years ago

Comment from mreynolds (@mreynolds389) at 2017-02-11 22:49:02

Metadata Update from @mreynolds389: