Re: [gentoo-portage-dev] [PATCH 5/5] FetchIterator: add terminate method

2017-03-24 Thread Zac Medico 
On Fri, Mar 24, 2017 at 1:05 PM, Brian Dolbec  wrote:
> On Thu, 23 Mar 2017 19:55:00 -0700
> Zac Medico  wrote:
>
>> Add a terminate method to FetchIterator so that it doesn't
>> delay termination of emirrordist via SIGINT. Otherwise it
>> it is possible for the __iter__ method to loop for a very
>> long time after SIGINT has been delivered. For example,
>> this could happen if there are many ebuilds with stale
>> cache and RESTRICT=mirror. This issue was discovered
>> during testing of changes to the MirrorDistTask.terminate
>> implementation.
>> ---
>>  pym/portage/_emirrordist/FetchIterator.py  | 21 +
>>  pym/portage/_emirrordist/MirrorDistTask.py |  6 +-
>>  2 files changed, 26 insertions(+), 1 deletion(-)
>>
>
> Not that I know enough about all this that I could say "your doing it
> wrong"
>
> But the series looks OK, I didn't see any obvious goofs ;)
>
> Thanks

Thanks, pushed:

https://gitweb.gentoo.org/proj/portage.git/commit/?id=04b1012594bfad1be719547e2c88a2dcf1051dc1
https://gitweb.gentoo.org/proj/portage.git/commit/?id=86400e9f864e86f8f677ccda9ce4103d6d02ef87
https://gitweb.gentoo.org/proj/portage.git/commit/?id=61878e4fbdfef5f8512b34640089e954a14e6d12
https://gitweb.gentoo.org/proj/portage.git/commit/?id=7defd54354c17afce7f36f53431260c1909481be
https://gitweb.gentoo.org/proj/portage.git/commit/?id=f70db92cba82535c8f65d385bd08a40207e24ec1
-- 
Thanks,
Zac

Re: [gentoo-portage-dev] [PATCH 5/5] FetchIterator: add terminate method

2017-03-24 Thread Brian Dolbec 
On Thu, 23 Mar 2017 19:55:00 -0700
Zac Medico  wrote:

> Add a terminate method to FetchIterator so that it doesn't
> delay termination of emirrordist via SIGINT. Otherwise it
> it is possible for the __iter__ method to loop for a very
> long time after SIGINT has been delivered. For example,
> this could happen if there are many ebuilds with stale
> cache and RESTRICT=mirror. This issue was discovered
> during testing of changes to the MirrorDistTask.terminate
> implementation.
> ---
>  pym/portage/_emirrordist/FetchIterator.py  | 21 +
>  pym/portage/_emirrordist/MirrorDistTask.py |  6 +-
>  2 files changed, 26 insertions(+), 1 deletion(-)
> 
> diff --git a/pym/portage/_emirrordist/FetchIterator.py
> b/pym/portage/_emirrordist/FetchIterator.py index 16a0b04..3841979
> 100644 --- a/pym/portage/_emirrordist/FetchIterator.py
> +++ b/pym/portage/_emirrordist/FetchIterator.py
> @@ -1,6 +1,8 @@
>  # Copyright 2013 Gentoo Foundation
>  # Distributed under the terms of the GNU General Public License v2
>  
> +import threading
> +
>  from portage import os
>  from portage.checksum import (_apply_hash_filter,
>   _filter_unaccelarated_hashes, _hash_filter)
> @@ -13,6 +15,19 @@ class FetchIterator(object):
>   def __init__(self, config):
>   self._config = config
>   self._log_failure = config.log_failure
> + self._terminated = threading.Event()
> +
> + def terminate(self):
> + """
> + Schedules early termination of the __iter__ method,
> which is
> + useful because under some conditions it's possible
> for __iter__
> + to loop for a long time without yielding to the
> caller. For
> + example, it's useful when there are many ebuilds
> with stale
> + cache and RESTRICT=mirror.
> +
> + This method is thread-safe (and safe for signal
> handlers).
> + """
> + self._terminated.set()
>  
>   def _iter_every_cp(self):
>   # List categories individually, in order to start
> yielding quicker, @@ -37,6 +52,9 @@ class FetchIterator(object):
>  
>   for cp in self._iter_every_cp():
>  
> + if self._terminated.is_set():
> + return
> +
>   for tree in portdb.porttrees:
>  
>   # Reset state so the Manifest is
> pulled once @@ -46,6 +64,9 @@ class FetchIterator(object):
>  
>   for cpv in portdb.cp_list(cp,
> mytree=tree): 
> + if self._terminated.is_set():
> + return
> +
>   try:
>   restrict, =
> portdb.aux_get(cpv, ("RESTRICT",), mytree=tree)
> diff --git a/pym/portage/_emirrordist/MirrorDistTask.py
> b/pym/portage/_emirrordist/MirrorDistTask.py index 0702eb1..8da9f06
> 100644 --- a/pym/portage/_emirrordist/MirrorDistTask.py
> +++ b/pym/portage/_emirrordist/MirrorDistTask.py
> @@ -32,9 +32,11 @@ class MirrorDistTask(CompositeTask):
>   self._config = config
>   self._term_rlock = threading.RLock()
>   self._term_callback_handle = None
> + self._fetch_iterator = None
>  
>   def _start(self):
> - fetch =
> TaskScheduler(iter(FetchIterator(self._config)),
> + self._fetch_iterator = FetchIterator(self._config)
> + fetch = TaskScheduler(iter(self._fetch_iterator),
>   max_jobs=self._config.options.jobs,
>   max_load=self._config.options.load_average,
>   event_loop=self._config.event_loop)
> @@ -226,6 +228,8 @@ class MirrorDistTask(CompositeTask):
>   self._term_callback)
>  
>   def _term_callback(self):
> + if self._fetch_iterator is not None:
> + self._fetch_iterator.terminate()
>   self.cancel()
>   self.wait()
>  


Not that I know enough about all this that I could say "your doing it
wrong"

But the series looks OK, I didn't see any obvious goofs ;)

Thanks

-- 
Brian Dolbec

[gentoo-portage-dev] [PATCH 5/5] FetchIterator: add terminate method

2017-03-23 Thread Zac Medico 
Add a terminate method to FetchIterator so that it doesn't
delay termination of emirrordist via SIGINT. Otherwise it
it is possible for the __iter__ method to loop for a very
long time after SIGINT has been delivered. For example,
this could happen if there are many ebuilds with stale
cache and RESTRICT=mirror. This issue was discovered
during testing of changes to the MirrorDistTask.terminate
implementation.
---
 pym/portage/_emirrordist/FetchIterator.py  | 21 +
 pym/portage/_emirrordist/MirrorDistTask.py |  6 +-
 2 files changed, 26 insertions(+), 1 deletion(-)

diff --git a/pym/portage/_emirrordist/FetchIterator.py 
b/pym/portage/_emirrordist/FetchIterator.py
index 16a0b04..3841979 100644
--- a/pym/portage/_emirrordist/FetchIterator.py
+++ b/pym/portage/_emirrordist/FetchIterator.py
@@ -1,6 +1,8 @@
 # Copyright 2013 Gentoo Foundation
 # Distributed under the terms of the GNU General Public License v2
 
+import threading
+
 from portage import os
 from portage.checksum import (_apply_hash_filter,
_filter_unaccelarated_hashes, _hash_filter)
@@ -13,6 +15,19 @@ class FetchIterator(object):
def __init__(self, config):
self._config = config
self._log_failure = config.log_failure
+   self._terminated = threading.Event()
+
+   def terminate(self):
+   """
+   Schedules early termination of the __iter__ method, which is
+   useful because under some conditions it's possible for __iter__
+   to loop for a long time without yielding to the caller. For
+   example, it's useful when there are many ebuilds with stale
+   cache and RESTRICT=mirror.
+
+   This method is thread-safe (and safe for signal handlers).
+   """
+   self._terminated.set()
 
def _iter_every_cp(self):
# List categories individually, in order to start yielding 
quicker,
@@ -37,6 +52,9 @@ class FetchIterator(object):
 
for cp in self._iter_every_cp():
 
+   if self._terminated.is_set():
+   return
+
for tree in portdb.porttrees:
 
# Reset state so the Manifest is pulled once
@@ -46,6 +64,9 @@ class FetchIterator(object):
 
for cpv in portdb.cp_list(cp, mytree=tree):
 
+   if self._terminated.is_set():
+   return
+
try:
restrict, = portdb.aux_get(cpv, 
("RESTRICT",),
mytree=tree)
diff --git a/pym/portage/_emirrordist/MirrorDistTask.py 
b/pym/portage/_emirrordist/MirrorDistTask.py
index 0702eb1..8da9f06 100644
--- a/pym/portage/_emirrordist/MirrorDistTask.py
+++ b/pym/portage/_emirrordist/MirrorDistTask.py
@@ -32,9 +32,11 @@ class MirrorDistTask(CompositeTask):
self._config = config
self._term_rlock = threading.RLock()
self._term_callback_handle = None
+   self._fetch_iterator = None
 
def _start(self):
-   fetch = TaskScheduler(iter(FetchIterator(self._config)),
+   self._fetch_iterator = FetchIterator(self._config)
+   fetch = TaskScheduler(iter(self._fetch_iterator),
max_jobs=self._config.options.jobs,
max_load=self._config.options.load_average,
event_loop=self._config.event_loop)
@@ -226,6 +228,8 @@ class MirrorDistTask(CompositeTask):
self._term_callback)
 
def _term_callback(self):
+   if self._fetch_iterator is not None:
+   self._fetch_iterator.terminate()
self.cancel()
self.wait()
 
-- 
2.10.2