Hi,
thanks to your reply, no problem :)
> * After some discussion with RobR, I think we'd prefer a little
> different hint structure:
>
> typedef struct
> {
> char * type; /* null terminated */
> char * hint;
> int length;
> } PVFS_hint;
> This essentially replaces the type with a string, allowing us to keep
> the external interfaces (and types) relatively simple and static (not
> changing). With an enum for the types, the external interfaces
> (namely the enum) changes everytime someone adds a new hint.
> Internally, having the enum of hints is fine, and converting to that
> format seems appropriate for sending the hints over the wire.
Ok that is a good point, I will make the changes soon.
> Also, I would prefer to see each of the system interfaces take an
> array of hints (pointer to PVFS_hint and length) instead of the
> linked list structure it is now. That would mean that the caller has
> to allocate the array himself instead of calling _add_hint, but I'm
> ok with that, esp. since for just one hint (most likely case) we can
> keep it on the stack instead of the heap.
I do not like that idea, I agree it would be ok to have a different structure
internally and one for the system interface. The conversion could be done at
the beginning of each system interface function. If we use for example a
linked list of the new hint with the type (char*, char*,length) then this
could be converted into the enum and array used internally.
> * All the PINT_* functions should be moved to internal headers or
> remove altogether. In fact, with just the type definition in the
> hints header, it probably makes sense to move that to pvfs2-types.h.
Ah I see, so you prefer to have a array to avoid something like add- or set
hint in conjunction with free hint ? I prefer the mpi approach but I will do
as the people like to see it...
> * I don't like the PINT_hint_add_environment_hints function. I can't
> think of any reasonable use cases where someone would want to set
> hints with environment variables, and it seems like adding it as a
> feature will allow users to shoot themselves in the foot.
I have a good example for you and I used it for some testing, too :)
Imagine the hint which sets the names and number of servers the datafiles of a
new file should be placed. It is interesting for some people to set something
like that for pvfs2-cp, pvfs2-touch etc. Instead of adding a new command
line parameter to all that tools it could be easily set with the
add_environment function, which I think is also nice if you want to test new
hints quickly. This of course is ok for parameters which are rarely needed
and useful for a lot of tools.
> In order to limit the length of the hints in the server request, can
> we only add the "well known" hints to the request? Well known hints
> would be the ones defined in the internal enum.
Mhh, we come back to this later, right now I thought it is a good idea to
limit the hints (by beeing forced to put the stuff into the enum) and also
force the user to specify if the hint should be transfered to the server in
the additional structure. (This limits the use cases to client only hints,
and client + server hints).
> * The request id format you're using seems rather inefficient. I'd
> prefer to see it be a 64 or 128 bit value, and have client-core just
> generate something like a uuid. It might be nice to have a util
> function that would generate the id that could be passed to the
> different system interfaces:
Yes it is rather inefficient, but I like it the way it is, because we do not
want to create request id's for something different than debugging/logging,
also the nice string representation allows us to get all the interesting
information about a particular request quickly. For example imagine that
there is a problem in Trove somewhere which is triggered only by a special
MPI request, whatever, like MPI_write_shared then you could simply print the
hint in Trove and you will see this request origin is MPI_write_shared and
this or that machine... I do not think it is a performance or bandwith
problem to encapsulate the string. It is also nice to identify information
easily with sniffing without decoding the pakage...
> PVFS_hint_generate(const char * type, PVFS_hint * hint);
> It seems like the server should be able to treat the request id as
> completely opaque, allowing us to change it later if necessary.
I think it is nice to encapsulate as many information as needed, but as
flexible... That's why I like to have it as a string...
> Are
> you adding the hostname and pid to the event logs?
we have to add the job id and rank, however a global unique identifier is also
ok.
> Would it be
> possible/difficult to extract that info from the id in the logfile at
> display time?
Yes it would be difficult. We do not get any information during the display
time from jumpshot except the info buffer which has to be prepared before....
> I'm on the fence on this though...maybe we should pick
> a format and stick with it?
<= has to be discussed when the other issues are resolved.
> * The code in your branch adds hint parameters to all the job, trove,
> bmi and flow calls. I'm still in favor of associating the hint with
> the job/trove/bmi ids in separate event calls.
We talked with Rob, and yeah the hint parameter given to job, trove, bmi and
flow calls can be seen as seperate patch which we will provide on top of the
cvs version. It is very convinient for us to have it in a branch or head :)
That means we will not provide a mapping or something in seperate event calls
and we use the direct method as parameter extension. Also if more people will
use the hint mechanism (or want to use it), it is easier to incooperate it
into the calls. For example it is hard to do a mapping if you have a
immediate completed call, then you can't add a mapping to the job_id because
none is created, also you might want to have the hint during the call....
>Actually it looks
> like none of the bmi stuff uses the hints at all (only trove and flow)?
That is true, however I already modified the stuff in the job layer to provide
it once we need it.
Julian
_______________________________________________
Pvfs2-developers mailing list
[email protected]
http://www.beowulf-underground.org/mailman/listinfo/pvfs2-developers
