Hi Matevz,
Whatever. This is more personal aesthetics that anything else. I have no
issues inheriting from XrdJob because I want my class, whatever it is, to be
associated with a job that does something with the class. Simple as that.
One of the tenets in the xrootd framework is that any class can be scheduled
and that's how it's done. If you rather use indirection that's OK as well
though as far as I'm concerned that's yet another malloc())-free() call that
simply is not needed and generally aggravates VM_ARENA issues in RH6.
Andy
-----Original Message-----
From: Matevz Tadel
Sent: Wednesday, March 26, 2014 7:30 AM
To: Lukasz Janyst ; Andrew Hanushevsky ; Alja Mrak-Tadel
Cc: xrootd-dev
Subject: Re: release status
Yup, sure ... I had the same feeling about this :)
Matevz
On 03/26/14 03:06, Lukasz Janyst wrote:
> I mean like:
>
> class DelayFileClose: public Job
> {
>
>
> };
>
> schedP->Schedule(new DelayFileClose(fP))
>
> Lukasz
>
> On 26.03.2014 10:16, Lukasz Janyst wrote:
>> On 26.03.2014 03:16, Andrew Hanushevsky wrote:
>>> 1) XrdPosixFile needs to inherit the XrdJob class.
>>
>> Ugh, that's ugly. File really is not a Job. What if you will need to
>> delay a different thing later on? Can this be wrapped somehow?
>>
>> Lukasz
>>
>>> 2) XrdPosixFile needs to define the virtual method DoIt() from XrdJob to
>>> simply call its DelayedDestroyed method and return.
>>> 3) When you need to delay the destroy then if the schedP exists,
>>> schedule the destroy via schedP->Schedule((XrdJob *)fP) otherwise run a
>>> separate thread as you do now.
>>>
>>> OK?
>>>
>>> Andy
>>>
>>> -----Original Message----- From: Alja Mrak-Tadel
>>> Sent: Tuesday, March 25, 2014 3:40 PM
>>> To: Andrew Hanushevsky ; Matevz Tadel
>>> Cc: xrootd-dev
>>> Subject: Re: release status
>>>
>>> Hi Andy,
>>>
>>> This is the minimal change that it does what we need
>>> https://github.com/alja/xrootd/compare/posix-close
>>>
>>> Alja
>>>
>>> On 03/24/14 16:23, Andrew Hanushevsky wrote:
>>>> Hi Matevz,
>>>>
>>>> The reason is that I chose ioActive() is simply that I thought we would
>>>> keep the code that destroys the posix object the same in close() and
>>>> execute it if we could do so immediately or schedule a thread to do a
>>>> defer close. In any case, the code (as it does now) at the very least
>>>> removes the FD from the fd table. Then again, looking at the code
>>>> changes I do get a little confused. I suppose I would say keep the old
>>>> logic as close as to the original with minimal changes to solve this.
>>>>
>>>> Andy
>>>>
>>>> On Mon, 24 Mar 2014, Matevz Tadel wrote:
>>>>
>>>>> Hi Andy,
>>>>>
>>>>> I vote for
>>>>>
>>>>> bool initiateClose();
>>>>>
>>>>> as this makes it clear that Close() will be called in a jiffy and that
>>>>> irreversible state changes might occur in the object. ioActive()
>>>>> sounds like something that can be called several times without
>>>>> changing the object.
>>>>>
>>>>> But ... you're the master so please just pick what you think is
>>>>> consistent with the rest.
>>>>>
>>>>> Matevz
>>>>>
>>>>> On 03/24/14 12:54, Andrew Hanushevsky wrote:
>>>>>> Hi Alja,
>>>>>>
>>>>>> Well, if that is the simplest way to do it then let's go ahead with
>>>>>> that. I
>>>>>> assume th I/O pending state is quite dynamic and can't really be
>>>>>> easily handled
>>>>>> by some simple flag. So, let's go with something like:
>>>>>>
>>>>>> bool ioActive();
>>>>>>
>>>>>> which returns true that triggers a delayed close and false which
>>>>>> allows the
>>>>>> caller to do an inline close. If there is more than just active I/O
>>>>>> that can
>>>>>> place the object is a non-closeable state then we should use a more
>>>>>> generic name
>>>>>> (e.g. isCloseable()). What do you think?
>>>>>>
>>>>>> Andy
>>>>>>
>>>>>> On Sun, 23 Mar 2014, Alja Mrak-Tadel wrote:
>>>>>>
>>>>>>> Hi Andy,
>>>>>>>> I was wondering if there was a way to structure this so that a
>>>>>>>> thread is run
>>>>>>>> to destroy the file object only if a file cache is being used. I'd
>>>>>>>> rather not
>>>>>>>> add the thread overhead to things like fuse and a non-caching proxy
>>>>>>>> server.
>>>>>>>> As for reusung a worker thread, while a good idea, we really don't
>>>>>>>> have
>>>>>>>> access to them. Something to think about. I'll see how difficult it
>>>>>>>> would be.
>>>>>>>> In any case, even if I could provide access to the thread pool,
>>>>>>>> there won't
>>>>>>>> be one if the posix interface is being used outside of xrootd, so
>>>>>>>> the code
>>>>>>>> would have to have two paths in any case.
>>>>>>>>
>>>>>>> How about adding a new virtual XrdOucCacheIO function that checks if
>>>>>>> destruction of XrdPosixFile needs to be delayed (e.g.
>>>>>>> InitiateClose(),
>>>>>>> RequiresDelayedClose(), TryClose() ...)? The return value of this
>>>>>>> function
>>>>>>> determines if a new thread is needed to run the real Close(). In
>>>>>>> this function
>>>>>>> I will prevent issuing any new read requests so it has to be
>>>>>>> guaranteed that
>>>>>>> the real Close() is called immediately after that.
>>>>>>>
>>>>>>> Alja
>>>>>>>
>>>>>>>> On Fri, 21 Mar 2014, Alja Mrak-Tadel wrote:
>>>>>>>>
>>>>>>>>> Hi Andy,
>>>>>>>>>
>>>>>>>>> I had to move destruction of XrdPosixFile into separate task to
>>>>>>>>> avoid long
>>>>>>>>> lock in XrdOfsHandle::Alloc() and XrdOfsHandle::Retire(). This is
>>>>>>>>> the change
>>>>>>>>> which fixed the problem:
>>>>>>>>> https://github.com/alja/xrootd/compare/pd
>>>>>>>>> You might want to reuse existing worker threads instead. One of
>>>>>>>>> the
>>>>>>>>> XrdClFile::Close() calls is also removed.
>>>>>>>>>
>>>>>>>>> We also made improvements in XrdFileCache module. We need a day or
>>>>>>>>> two to
>>>>>>>>> close up changes and test them.
>>>>>>>>>
>>>>>>>>> Alja
>>>>>>>>>
>>>>>>>>>
>>>>>>>>>
>>>>>>>>> On 03/20/14 13:33, Andrew Hanushevsky wrote:
>>>>>>>>>> Hi Matevz,
>>>>>>>>>>
>>>>>>>>>> Hmmmm, ok you need to give me specific details on what you want
>>>>>>>>>> done.
>>>>>>>>>>
>>>>>>>>>> Andy
>>>>>>>>>>
>>>>>>>>>> On Thu, 20 Mar 2014, Matevz Tadel wrote:
>>>>>>>>>>
>>>>>>>>>>> Hi,
>>>>>>>>>>>
>>>>>>>>>>> Alja and I would like to push in some changes for the
>>>>>>>>>>> XrdFileCache. We
>>>>>>>>>>> have our own workaround for destruction of XrdPosixFile /
>>>>>>>>>>> XrdOucCacheIO in a special thread ... Andy will probably want
>>>>>>>>>>> to
>>>>>>>>>>> implement this for us so he stays on top of things.
>>>>>>>>>>>
>>>>>>>>>>> We should be good to go early next week.
>>>>>>>>>>>
>>>>>>>>>>> Cheers,
>>>>>>>>>>> Matevz
>>>>>>>>>>>
>>>>>>>>>>>
>>>>>>>>>>> On 03/20/14 10:04, Lukasz Janyst wrote:
>>>>>>>>>>>> Hello everyone,
>>>>>>>>>>>>
>>>>>>>>>>>> I have done all the coding that was on my list. I have two
>>>>>>>>>>>> items
>>>>>>>>>>>> left:
>>>>>>>>>>>>
>>>>>>>>>>>> 1) packaging - in the process of being done - consulting with
>>>>>>>>>>>> Mattias to
>>>>>>>>>>>> integrate with EPEL. I have some draft specfiles, but probably
>>>>>>>>>>>> need
>>>>>>>>>>>> two or three
>>>>>>>>>>>> more iterations.
>>>>>>>>>>>>
>>>>>>>>>>>> 2) the server is broken when compiled in C++11 mode. It didn't
>>>>>>>>>>>> seem
>>>>>>>>>>>> to respond
>>>>>>>>>>>> when I tested it three months ago, so perhaps the issue is
>>>>>>>>>>>> fixed now,
>>>>>>>>>>>> needs
>>>>>>>>>>>> testing. Andy, can you please check whether this is still the
>>>>>>>>>>>> case?
>>>>>>>>>>>> You can use
>>>>>>>>>>>> my testing cluster.
>>>>>>>>>>>>
>>>>>>>>>>>> Cheers,
>>>>>>>>>>>> Lukasz
>>>>>>>>>>>>
>>>>>>>>>>>> ########################################################################
>>>>>>>>>>>>
>>>>>>>>>>>>
>>>>>>>>>>>>
>>>>>>>>>>>>
>>>>>>>>>>>> Use REPLY-ALL to reply to list
>>>>>>>>>>>>
>>>>>>>>>>>> To unsubscribe from the XROOTD-DEV list, click the following
>>>>>>>>>>>> link:
>>>>>>>>>>>> https://listserv.slac.stanford.edu/cgi-bin/wa?SUBED1=XROOTD-DEV&A
>>>>>>>>>>>>
>>>>>>>>>>>
>>>>>>>>>>> ########################################################################
>>>>>>>>>>>
>>>>>>>>>>>
>>>>>>>>>>>
>>>>>>>>>>> Use REPLY-ALL to reply to list
>>>>>>>>>>>
>>>>>>>>>>> To unsubscribe from the XROOTD-DEV list, click the following
>>>>>>>>>>> link:
>>>>>>>>>>> https://listserv.slac.stanford.edu/cgi-bin/wa?SUBED1=XROOTD-DEV&A=1
>>>>>>>>>>>
>>>>>>>>>>>
>>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>> ########################################################################
>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>> Use REPLY-ALL to reply to list
>>>>>>>>>>
>>>>>>>>>> To unsubscribe from the XROOTD-DEV list, click the following
>>>>>>>>>> link:
>>>>>>>>>> https://listserv.slac.stanford.edu/cgi-bin/wa?SUBED1=XROOTD-DEV&A=1
>>>>>>>>>>
>>>>>>>>>
>>>>>>>>> ########################################################################
>>>>>>>>>
>>>>>>>>>
>>>>>>>>>
>>>>>>>>> Use REPLY-ALL to reply to list
>>>>>>>>>
>>>>>>>>> To unsubscribe from the XROOTD-DEV list, click the following link:
>>>>>>>>> https://listserv.slac.stanford.edu/cgi-bin/wa?SUBED1=XROOTD-DEV&A=1
>>>>>>>>>
>>>>>>>>
>>>>>>>> ########################################################################
>>>>>>>>
>>>>>>>>
>>>>>>>>
>>>>>>>> Use REPLY-ALL to reply to list
>>>>>>>>
>>>>>>>> To unsubscribe from the XROOTD-DEV list, click the following link:
>>>>>>>> https://listserv.slac.stanford.edu/cgi-bin/wa?SUBED1=XROOTD-DEV&A=1
>>>>>>>
>>>>>
>>>>> ########################################################################
>>>>>
>>>>> Use REPLY-ALL to reply to list
>>>>>
>>>>> To unsubscribe from the XROOTD-DEV list, click the following link:
>>>>> https://listserv.slac.stanford.edu/cgi-bin/wa?SUBED1=XROOTD-DEV&A=1
>>>>>
>>>
>>> ########################################################################
>>> Use REPLY-ALL to reply to list
>>>
>>> To unsubscribe from the XROOTD-DEV list, click the following link:
>>> https://listserv.slac.stanford.edu/cgi-bin/wa?SUBED1=XROOTD-DEV&A=1
>>
>
########################################################################
Use REPLY-ALL to reply to list
To unsubscribe from the XROOTD-DEV list, click the following link:
https://listserv.slac.stanford.edu/cgi-bin/wa?SUBED1=XROOTD-DEV&A=1
|