[Openais] patch AMF sync

[Steven Dake]

I think you are fine with commit with other issues fixed.

Regards
-steve
On Fri, 2006-08-11 at 08:17 +0200, Hans Feldt wrote:
> See comments below. If the usleep is removed and other issues fixed, can 
> I commit or do you want to see a new patch?
> 
> Regards,
> Hans
> 
> Steven Dake wrote:
> > Hans,
> > 
> > The patch looks pretty good.  I have the following comments:
> > 1) amf_csi_assignment_find should be broken up if possible.
> > 2) amf_csi_attribute_serialize there is a comment which doesn't follow
> > coding standards and is not tab aligned
> > 3) in amfsu.c line 214 csi_assignment variable is not checked for
> > invalid allocation
> > 4) amf_su_new comment doesn't follow coding standards
> > 5) why is AMF_RESPONSE_COMPONENTTERMINATECALLBACK case commented out?
> 
> Not used/tested yet I guess? Will be fixed shortly.
> 
> > 6) node_leaved is not proper english, should be node_left
> > 7) absolutely no sleeping is allowed in the executive in sync_request
> > this must be fixed.  I suggest a token callback which will be executed
> > every time the token has returned which may be an opportunity to execute
> > more process requests.  Better yet use the sync infrastructure, or if it
> > is not suitable, tell me why and I'll fix it.
> 
> I realize now that it is stupid. But this is the sync_request function 
> that I want you to implement properly in the SYNC service as described 
> in earlier mails.
> 
> If I replace usleep with error handling instead until sync_request is 
> inplace, is that OK for now?
> 
> > 8) SYNCHRONISING should be spelled SYNCHRONIZING
> > 9) non-conformant comment: +    /* not designed to handle more than one
> > leaving node */
> > 10) I suggest using the regular openais_timer_add functions instead of
> > poll_timer_add.  If these functions have problems (which I think have
> > been addressed now) then I'd like to know about them so they can be
> > fixed.  the poll timer add should only be used by totem.
> 
> OK.
> 
> > 11) please change this code to follow coding standards:
> > +               req_exec.error     = req_lib->error;
> > +               iovec.iov_base     = (char *)&req_exec;
> > +               iovec.iov_len      = sizeof (req_exec);
> > 
> > Overall the code looks great and the only real show-stopper on commit is
> > the usleep that is used in sync_request.
> > 
> > Regards
> > -steve
> > 
> > On Thu, 2006-08-10 at 17:51 +0200, Hans Feldt wrote:
> > 
> >>Here it is at last. The patch includes AMF sync support but _not_ node
> >>failover support which will be next thing. No ASN.1 coding has been used
> >>but an own XDR similar encoding for now.
> >>
> >>Summary:
> >>===========
> >>- New sync state machine, implemented and described in amf.c
> >>- One AMF node reads the AMF config file (IMM style)
> >>- One AMF node syncs others AMF nodes
> >>- One AMF object is serialized and sent as one message
> >>- Serialization/deserialization of most objects is trivial (memcpy)
> >>except for component and csi-attributes objects which have variable size
> >>arrays/strings.
> >>- Depth first AMF object tree traversal preserves relations when syncing
> >>- Ordered lists of SUs and SIs
> >>- Constructors/destructor per class
> >>- Serializers/deserializers per class
> >>- Config-change changes sync state
> >>- Sync callbacks executes the sync
> >>- "Use case" tracing for sync using the SYNCTRACE macro (trace6)
> >>- Sync master is initially the winner of a timeout race and if the
> >>master leaves the cluster, the node with the lowest node ID becomes new 
> >>master.
> >>- amf_malloc implements an AMF central malloc routine with error handling.
> >>
> >>TODO:
> >>========
> >>- sync_abort support needed from the SYNC service.
> >>- sync_request support needed from the SYNC service.
> >>- Split amf.h into several files
> >>- AMF subdir
> >>- Node join/leave functionality
> >>- Work on serializers/deserializers
> >>
> >>Steven, I would like to commit this tomorrow since I will be away for a 
> >>few days. So if you could glance through...
> >>
> >>Regards,
> >>Hans
> >>
> >>
> >>_______________________________________________
> >>Openais mailing list
> >>Openais at lists.osdl.org
> >>https://lists.osdl.org/mailman/listinfo/openais
> > 
> > 
> > 
>