Print

Print


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