[asterisk-bugs] [Asterisk 0010668]: Incomplete CDR lock

[noreply at bugs.digium.com]

A NOTE has been added to this issue. 
====================================================================== 
http://bugs.digium.com/view.php?id=10668 
====================================================================== 
Reported By:                arkadia
Assigned To:                murf
====================================================================== 
Project:                    Asterisk
Issue ID:                   10668
Category:                   CDR/General
Reproducibility:            always
Severity:                   minor
Priority:                   normal
Status:                     feedback
Asterisk Version:           1.4.11  
SVN Branch (only for SVN checkouts, not tarball releases):  1.4  
SVN Revision (number only!):  
Disclaimer on File?:        N/A 
Request Review:              
====================================================================== 
Date Submitted:             09-07-2007 08:26 CDT
Last Modified:              06-05-2008 18:43 CDT
====================================================================== 
Summary:                    Incomplete CDR lock
Description: 
This is all about the meaning of AST_CDR_FLAG_LOCKED flag for CDR. 
I assume if CDR has this flag set then it wont be updated anymore by any
ast_cdr_ functions except cases when we force such update. ast_cdr_reset()
is example of function which allows to update locked CDRs if special flag
is given.

Based on this assumption I prepared the patch with such changes:

  ast_cdr_setvar() - not allowed to touch locked CDRs. I don't see any
cases when we need to change locked CDRs too. Not touching locked CDRs is
helpfull when ForkCDR being used and every new CDR may have different
custom variables.

  ast_cdr_answer() - don't change status of locked CDRs. That might be
helpfull when we have a chain of CDRs for one incoming call for which we
perform several tries to make outgoing call. Every outgoing attempt may
finish with its own completion code. And if the final attempt is 'answered'
then we have only one CDR with 'answered' status. All other will keep
connection error codes which is very helpful for error monitoring.

 ast_cdr_end() - don't change locked CDRs by the same reason as
ast_cdr_answer()

As there will be no changes in CDR after lock, app_forkcdr.c should be
updated to end original locked CDR.
Also it'll be great to make optional cdr reset in this application, but
thats a theme for another patch.
======================================================================
Relationships       ID      Summary
----------------------------------------------------------------------
related to          0012726 ForkCDR application fails to set duration
====================================================================== 

---------------------------------------------------------------------- 
 murf - 06-05-08 18:43  
---------------------------------------------------------------------- 
Sorry, but all hope of an API consistency argument is gone. The API was
never 
documented, so for years, everyone made their own mental API based on its

behavior, which forms an 'implicit' documentation. And that unpublished,
but
ubiquitous mental document based on experimentation, trial, and error is
the
standard. There is no way you can change the behavior of these funcs and
get
it merged into the code. Any changes you would like to make to solve your
problem must not disturb the current behavior. So, the API must stay as it
is.

> Also somebody may want to lock not ended CDR by some reason.

They can file a bug, we'll make a new option, and they can achieve this
that way.

> AST_CDR_FLAG_LOCKED is a part of public API used by developers in their
> applications. Preferably this flag has constant and clear meaning like:

> if CDR is marked with this flag it is not updated by any ast_cdr_.... 
> functions.

Sorry, but this is wrong. AST_CDR_FLAG_LOCKED is NOT part of a public
API.
It's only minimally documented in the source code. The way it's used
right
now is not up for negotiation. People have tons of code built around the
way things work. Any tweaks that need to be made, need to be via new
options,
and must be transparent to current implementations.

Also, implementing new funcs and apps are definitely out. First of all,
they
complicate the picture, by adding new but similar apps to the existing
interface. Users will be confused about which to use in new
implementations.
Next, new apps are an enhancement, and are not on the table for discussion
for
1.4; If you want new stuff in trunk, we can talk, but if you want this in
1.4,
then we have to do it this way. Splitting up the LOCKED flag into 3 flags
is
almost what I've done. I've intro'd a new flag for locking out changes to
disposition, and an option to set that flag, basically. I could do it for
end, but I'll wait till someone actually needs it before I'll add it. For

setvar, we have enough flags to cover most situations. the 'l' flag 
will force a var set to be done only on the last CDR in the chain. 
This is basically the CDR created by a previous call to ForkCDR(). 
So, you can get what you need that way.

So, given these restrictions, I ask again, will the proposed changes give
you
what you need? 

Issue History 
Date Modified   Username       Field                    Change               
====================================================================== 
06-05-08 18:43  murf           Note Added: 0087874                          
======================================================================