Hi, FLIP has been updated with last comment On Thu, Jun 5, 2025 at 6:35 PM Gabor Somogyi <gabor.g.somo...@gmail.com> wrote:
> The interface has been added. Do we intend to pass an > instance registerWatchedPath function as we discussed? > If yes then the FLIP needs further adjustments all places where now > Callable provided. > > G > > > On Thu, Jun 5, 2025 at 2:27 PM Nicolas Fraison > <nicolas.frai...@datadoghq.com.invalid> wrote: > > > Hi, > > > > It indeed make sense, FLIP has been updated > > > > Nicolas > > > > On Wed, Jun 4, 2025 at 12:10 PM Gabor Somogyi <gabor.g.somo...@gmail.com > > > > wrote: > > > > > Hi, > > > > > > I've read it through. Basically looks good with one comment. > > > *registerWatchedPath function* has a *Callable* as callback. For the > > > current implementation that would be enough, > > > but when somebody would like to use that for any other use-case then it > > > would be hard. Examples: > > > * A single *call* function tells the user nothing what kind of event is > > > this > > > * the watch service supports 3 events (create, modify, delete), now > when > > I > > > register I can get only updates (I presume) > > > * 1+ dir watch with the same callback is not possible > > > > > > My suggestion would be to create a proper callback with all the event > > type > > > functions and no-op default behavior with the following names[1]. > > > This is ~30 lines addition but will increase the readability heavily + > no > > > need to touch this code later. > > > > > > BR, > > > G > > > > > > [1] > > > > > > > > > https://github.com/apache/flink-kubernetes-operator/commit/e5a325c48965a50d61d0aa29e61ba79e97f27082#diff-a30b3ed9b8c53e998b15d7da7ad2e54374c98ffc3c920f76a70bce3fb37a9b2eR87-R93 > > > > > > > > > On Tue, Jun 3, 2025 at 8:53 AM Nicolas Fraison > > > <nicolas.frai...@datadoghq.com.invalid> wrote: > > > > > > > Hi, > > > > > > > > The FLIP has been updated. > > > > Let me know if you have some other comments. > > > > > > > > Nicolas > > > > > > > > On Tue, May 20, 2025 at 12:01 PM Nicolas Fraison < > > > > nicolas.frai...@datadoghq.com> wrote: > > > > > > > > > Hi Gabor, > > > > > > > > > > Thanks for the feedback. > > > > > The overall proposal with atomic dirty flag being set by the > callback > > > > will > > > > > indeed work with any kind of implementation. > > > > > > > > > > Will see to update the FLIP in a week or 2 if there are no other > > > > comments > > > > > on it > > > > > > > > > > Nicolas > > > > > > > > > > > > > > > On Tue, May 20, 2025 at 11:08 AM Gabor Somogyi < > > > > gabor.g.somo...@gmail.com> > > > > > wrote: > > > > > > > > > >> Hi Nicolas, > > > > >> > > > > >> Related SSLContext I've not gone through all the cases where we > need > > > to > > > > >> reload so instead I'm sharing the concept. > > > > >> The main intention is that we must control all execution paths > which > > > > >> decide > > > > >> which certificate used for authentication. > > > > >> Creating an SSLContext decorator which checks reload first and > then > > > > >> forwards all calls to the original (wrapped) context > > > > >> is one way to achieve that. If there are different implementations > > > which > > > > >> end up in similar behavior then it's fine. > > > > >> > > > > >> BR, > > > > >> G > > > > >> > > > > >> > > > > >> On Tue, May 20, 2025 at 10:15 AM Nicolas Fraison > > > > >> <nicolas.frai...@datadoghq.com.invalid> wrote: > > > > >> > > > > >> > Hi, > > > > >> > > > > > >> > Overall your proposal looks great. > > > > >> > The event handling must indeed be super fast and we must also > not > > > > change > > > > >> > original code path if reload not needed > > > > >> > > > > > >> > I have some concerns around the ReloadableSSLContext > implementing > > > all > > > > >> > SSLContext > > > > >> > Do you really mean SSLContext (java SSLContext one) or do you > > refer > > > to > > > > >> the > > > > >> > SslContext from netty? > > > > >> > > > > > >> > If java SSLContext > > > > >> > - I'm not sure how this will manage reload from the BlobServer > > > > >> > BlobServer relies on creation of an SSLServerSocketFactory from > > the > > > > >> > SSLContext. > > > > >> > But from my current understanding the SSLServerSocketFactory > does > > > not > > > > >> have > > > > >> > any connection with the SSLContext. > > > > >> > It only has some with an SSLContextImpl extends SSLContextSpi. > > > > >> > I think we will need a callback here to enforce recreation of > the > > > > >> > BlobServer socket. > > > > >> > - I'm also don't see how to attach this to the SslContext from > > netty > > > > >> > > > > > >> > If netty SslContext > > > > >> > - we would still need to have the callback to recreate the > > > BlobServer > > > > >> > socket > > > > >> > - for netty and pekko we should be able to rely on it > > > > >> > > > > > >> > Nicolas > > > > >> > > > > > >> > On Wed, May 7, 2025 at 12:40 PM Gabor Somogyi < > > > > >> gabor.g.somo...@gmail.com> > > > > >> > wrote: > > > > >> > > > > > >> > > Hi All, > > > > >> > > > > > > >> > > I've read through the concerns/proposals related the watch > > service > > > > and > > > > >> > here > > > > >> > > are my conclusions: > > > > >> > > * Watch service can watch local file systems only: It's fair > to > > > say > > > > >> that > > > > >> > > certificates must be copied to local FS in order to work (init > > > > >> container > > > > >> > cp > > > > >> > > command or something) > > > > >> > > * Watch service can send multiple events even for a single > > > directory > > > > >> > > change: this can be mitigated with a single atomic dirty flag > > (see > > > > my > > > > >> > > design suggestion) > > > > >> > > * Polling file modification time: When I hear any kind of > > polling > > > > >> > > implemented by us is just something I'm mostly opposing > > > > >> > > * security.ssl.internal.keystore.reload.duration: Security > > > features > > > > >> must > > > > >> > be > > > > >> > > executed nearly immediately no matter what > > > > >> > > > > > > >> > > As a general remark, not sure how many users are using the > watch > > > > >> service > > > > >> > > based approach in the operator but until now I've not seen any > > > issue > > > > >> > > related to that. > > > > >> > > If somebody is having some specifics then please share. > > > > >> > > > > > > >> > > For now I would shoot for keystore not to have feature creep. > > When > > > > >> there > > > > >> > is > > > > >> > > a valid use-case, community interest and the keystore story is > > > > already > > > > >> > rock > > > > >> > > stable > > > > >> > > then we can consider to involve truststore later. > > > > >> > > > > > > >> > > Having the mentioned assumption that the operator approach > > works, > > > > >> here is > > > > >> > > my high level proposal: > > > > >> > > * Let's have enable flag for all such watch functionality. If > > it's > > > > >> false > > > > >> > > then the currently existing functionality must remain as-is > > > > >> > > * Let's have a LocalFSWatchService which is a singleton which > > has > > > no > > > > >> path > > > > >> > > registrations by default > > > > >> > > * Add path registration functionality which is synchronised > > where > > > a > > > > >> > > callback can be registered > > > > >> > > * Let's have a ReloadableSSLContext which implements the > > mentioned > > > > >> > callback > > > > >> > > * Inside the callback set an atomic dirty flag only (this can > > > handle > > > > >> > > multiple events for the same directory change + event handling > > in > > > > the > > > > >> > watch > > > > >> > > service must be extreme fast) > > > > >> > > * Inside ReloadableSSLContext all SSLContext actions must be > > > > >> overridden. > > > > >> > At > > > > >> > > the beginning of each function dirty flag must be checked > > > > >> > > and if dirty then certificates must be reloaded, flag can be > set > > > > back > > > > >> to > > > > >> > > false (then original functionality call). It's extremely > > important > > > > >> that > > > > >> > > context reload must be synchronised. > > > > >> > > A synchronised boolean check + possible context reload can > > consume > > > > >> some > > > > >> > > time but I wouldn't expect any significant performance drop. > > > > >> > > > > > > >> > > My proposal main drivers: > > > > >> > > * Original code path must run when no watch service asked > > > > >> > > * Super fast event handling because million events may come in > > > (not > > > > >> > > expecting but we should be prepared) > > > > >> > > * Clean separation between dir/file watch and file/dir usage > > > > >> > > * Well considered synchronisation model > > > > >> > > * Extensive unit testing because we're intended to touch the > > heart > > > > of > > > > >> > Flink > > > > >> > > > > > > >> > > Happy to hear other opinions. > > > > >> > > > > > > >> > > BR, > > > > >> > > G > > > > >> > > > > > > >> > > > > > > >> > > On Mon, Apr 28, 2025 at 1:54 PM Nicolas Fraison > > > > >> > > <nicolas.frai...@datadoghq.com.invalid> wrote: > > > > >> > > > > > > >> > > > Thanks all for your feedback and sorry for the late answer > (I > > > was > > > > on > > > > >> > > > holiday). > > > > >> > > > > > > > >> > > > 1. Indeed it would add 4 threads. > > > > >> > > > For the non pekko component we can indeed have one watcher > > > service > > > > >> used > > > > >> > > to > > > > >> > > > reload SSLContext for those components > > > > >> > > > For pekko this is a little more challenging as the creation > of > > > the > > > > >> > pekko > > > > >> > > > ssl engine is managed by pekko himself. > > > > >> > > > Flink only generates appropriate config with class to > execute > > to > > > > >> > initiate > > > > >> > > > the pekko ssl engine [1]. This means that I will not be able > > to > > > > >> provide > > > > >> > > the > > > > >> > > > watcher service to this ssl engine. > > > > >> > > > One solution would be to rely on a singleton instead of a > > > service > > > > >> > > injected > > > > >> > > > in each component but I'm not sure this is fine to use such > in > > > > >> flink. > > > > >> > > > WDYT? > > > > >> > > > > > > > >> > > > We can also add a specific flink configuration > > > > >> > > > (security.ssl.internal.keystore.reload.enable) to only add > > this > > > > >> watcher > > > > >> > > > mechanism if the config is enabled to avoid adding those > > threads > > > > if > > > > >> > this > > > > >> > > is > > > > >> > > > not needed. > > > > >> > > > > > > > >> > > > > > > > >> > > > 2. I'm fine with the LocalFSWatchService naming. > > > > >> > > > > > > > >> > > > 3. Also agree that some e2e tests must be added. > > > > >> > > > > > > > >> > > > > > > > >> > > > @doguscan namal, Thanks for challenging the proposal. > > > > >> > > > FYI, we are planning to rely on certificates with really > short > > > > >> > validity. > > > > >> > > > > > > > >> > > > D1. It seems that the proposal to rely on a reload period > will > > > > still > > > > >> > face > > > > >> > > > potential issues: > > > > >> > > > - receiving events while file content updates are still in > > > > progress: > > > > >> > > > There is no guarantee that we will not load the certificate > > > while > > > > >> file > > > > >> > > > content updates are still in progress > > > > >> > > > - while it will not be affected by multiple notifications, > we > > > can > > > > >> > reach a > > > > >> > > > point where only the truststore is updated when the reload > > > > happens. > > > > >> > > > Which means that if the keystore is updated just after, it > > will > > > > not > > > > >> be > > > > >> > > > taken in account before next run of the reload mechanism > > > > >> > > > > > > > >> > > > I think that with WatchService and appropriate reload grace > > > period > > > > >> > > > mechanism we should be able to mitigate those 2 issues > > (ensuring > > > > >> > minimum > > > > >> > > > reload even with multiple notify) > > > > >> > > > > > > > >> > > > From KIP-1119 [2] it looks like the same kind of > requirements > > is > > > > >> under > > > > >> > > > discussion for Kafka to also rely on the WatchService Java > API > > > > >> > > (SpringBoot > > > > >> > > > seems to also rely on this API to manage ssl reload [3]). > > > > >> > > > > > > > >> > > > D3. Do we have a real use case for this? > > > > >> > > > > > > > >> > > > [1] > > > > >> > > > > > > > >> > > > > > > > >> > > > > > > >> > > > > > >> > > > > > > > > > > https://github.com/apache/flink/blob/b523264ab45d37cd9584a0e8c06f1ef6bd1aaed7/flink-rpc/flink-rpc-akka/src/main/java/org/apache/flink/runtime/rpc/pekko/PekkoUtils.java#L372 > > > > >> > > > > > > > >> > > > [2] > > > > >> > > > > > > > >> > > > > > > > >> > > > > > > >> > > > > > >> > > > > > > > > > > https://cwiki.apache.org/confluence/display/KAFKA/KIP-1119%3A+Add+support+for+SSL+auto+reload > > > > >> > > > > > > > >> > > > [3] > > > > >> > > > > > > > >> > > > > > > > >> > > > > > > >> > > > > > >> > > > > > > > > > > https://docs.spring.io/spring-boot/reference/features/ssl.html#features.ssl.reloading > > > > >> > > > > > > > >> > > > Regards, > > > > >> > > > Nicolas > > > > >> > > > > > > > >> > > > On Wed, Apr 23, 2025 at 12:14 PM Doğuşcan Namal < > > > > >> > > namal.dogus...@gmail.com> > > > > >> > > > wrote: > > > > >> > > > > > > > >> > > > > Hi Nicolas, thanks for the FLIP. > > > > >> > > > > > > > > >> > > > > I am fully supportive of the motivation and we should be > > > > >> supporting > > > > >> > > this > > > > >> > > > > feature. Here are couple of comments from my side: > > > > >> > > > > > > > > >> > > > > D1) > > > > >> > > > > Since you shared the implementation details on the FLIP as > > > > well, I > > > > >> > > would > > > > >> > > > > like to discuss whether using Java's WatchService is the > > best > > > > >> choice > > > > >> > > > here. > > > > >> > > > > > > > > >> > > > > I believe that the certificate renewal is not a frequent > > > > >> operation. > > > > >> > > Even > > > > >> > > > > once a day certificate renewals are not realistic(except > for > > > > test > > > > >> > cases > > > > >> > > > > maybe) but let's assume that this covers up the p99.99 of > > the > > > > use > > > > >> > > cases. > > > > >> > > > I > > > > >> > > > > am confident on this estimation since there hasn't been a > > > > request > > > > >> > from > > > > >> > > > the > > > > >> > > > > community for this feature so far, which confirms that > > people > > > > were > > > > >> > okay > > > > >> > > > > with infrequent cluster restarts. Following that it is > > > > >> infrequent, I > > > > >> > > > > believe that spawning up a thread that watches the file > > > > >> modification > > > > >> > > > > operations is not the best use of the limited resources > on a > > > > >> cluster. > > > > >> > > > > > > > > >> > > > > There are some known limitations of the WatchService as > well > > > > such > > > > >> as > > > > >> > > > > receiving multiple modification events for the same > > occurence > > > > [1], > > > > >> > > > > inotify's (WatchService's underlying mechanism in Linux > > > > >> environments) > > > > >> > > > > problems on containerized environments due to remote file > > > > systems > > > > >> [2] > > > > >> > > or > > > > >> > > > > receiving events while file content updates are still in > > > > progress. > > > > >> > > [3]. I > > > > >> > > > > do not know if these limitations are addressed in the > newer > > > > >> versions > > > > >> > > but > > > > >> > > > > regardless of that it is clear that we may face with some > > ugly > > > > >> edge > > > > >> > > cases > > > > >> > > > > due to that. > > > > >> > > > > > > > > >> > > > > Given these complications, I would recommend just > creating a > > > new > > > > >> > > > SSLContext > > > > >> > > > > after a configured duration is expired. We could record > the > > > > >> timestamp > > > > >> > > > when > > > > >> > > > > the previous SSLContext is created and update it after a > > > > >> configured > > > > >> > > > > duration is passed. This will be much easier to test and > > > reason > > > > >> about > > > > >> > > > when > > > > >> > > > > it is running on production. This will eliminate the > > necessity > > > > to > > > > >> > > reason > > > > >> > > > > about the file modification operations as well. > > > > >> > > > > > > > > >> > > > > I briefly skimmed through the classes that need to be > > modified > > > > >> and it > > > > >> > > > > looked feasible for me. Let me know what are your comments > > on > > > > >> these. > > > > >> > > > > > > > > >> > > > > Note that this is already used in the Kafka world where a > > new > > > > >> > > SSLContext > > > > >> > > > is > > > > >> > > > > created after 12 hours. > > > > >> > > > > > > > > >> > > > > D2) We could provide a configuration to the user, such as > > > > >> > > > > "security.ssl.internal.keystore.reload.duration" so they > > could > > > > >> decide > > > > >> > > how > > > > >> > > > > often the new certificates should be loaded. > > > > >> > > > > > > > > >> > > > > D3) > > > > >> > > > > On the other hand, I wonder whether we should also handle > > > > >> supporting > > > > >> > > > > updating the file paths of the truststores and keystores > > under > > > > >> this > > > > >> > > FLIP > > > > >> > > > as > > > > >> > > > > well. Since the name of the FLIP is "Handle TLS > Certificate > > > > >> Renewal" > > > > >> > I > > > > >> > > > > think we could bring that into scope too :) > > > > >> > > > > > > > > >> > > > > Thanks > > > > >> > > > > > > > > >> > > > > [1] > > > > >> > > > > > > > > >> > > > > > > > > >> > > > > > > > >> > > > > > > >> > > > > > >> > > > > > > > > > > https://stackoverflow.com/questions/16777869/java-7-watchservice-ignoring-multiple-occurrences-of-the-same-event > > > > >> > > > > > > > > >> > > > > [2] > > > > https://blog.arkey.fr/2019/09/13/watchservice-and-bind-mount/ > > > > >> > > > > > > > > >> > > > > [3] > > > > >> > > > > > > > >> > > https://surajatreyac.github.io/2014-07-29/reactive_file_handling.html > > > > >> > > > > > > > > >> > > > > [4] See also Platform Dependencies - > > > > >> > > > > > > > > >> > > > > > > > > >> > > > > > > > >> > > > > > > >> > > > > > >> > > > > > > > > > > https://docs.oracle.com/javase/8/docs/api/index.html?java/nio/file/WatchService.html > > > > >> > > > > > > > > >> > > > > > > > > >> > > > > On Fri, 18 Apr 2025 at 18:25, Gabor Somogyi < > > > > >> > gabor.g.somo...@gmail.com > > > > >> > > > > > > > >> > > > > wrote: > > > > >> > > > > > > > > >> > > > > > Hi Robert, > > > > >> > > > > > > > > > >> > > > > > Since I've added the same feature to the operator I'll > > take > > > a > > > > >> look > > > > >> > at > > > > >> > > > it. > > > > >> > > > > > Though it won't be lightning fast since I'm having > several > > > > weeks > > > > >> > off. > > > > >> > > > > > > > > > >> > > > > > Your questions are valid especially considering the fact > > > that > > > > >> this > > > > >> > > > > feature > > > > >> > > > > > touches the hearth of the authentication so this must be > > > rock > > > > >> solid > > > > >> > > > > > in order to avoid grey hair :) > > > > >> > > > > > > > > > >> > > > > > (1) I would vote on a single service which is heavily > unit > > > > >> tested > > > > >> > > with > > > > >> > > > > > all the possible combinations including threading. > > > > >> > > > > > > > > > >> > > > > > Some standalone app could be added to really play with > it > > > > (that > > > > >> > would > > > > >> > > > > help > > > > >> > > > > > review). > > > > >> > > > > > I mean, create X files, start Y threads, and make > > > assertions. > > > > >> > > > > > The reason why I'm suggesting it is the fact that AFAIR > > the > > > > >> watch > > > > >> > > > service > > > > >> > > > > > is quite sensitive even in single thread. If we could do > > > this > > > > >> in a > > > > >> > > > finite > > > > >> > > > > > time > > > > >> > > > > > consuming unit test then it's even better. > > > > >> > > > > > > > > > >> > > > > > (2) +1 on that name to avoid confusion > > > > >> > > > > > > > > > >> > > > > > (3) I agree that some e2e is must, however this can be > > > easily > > > > >> and > > > > >> > > > deeply > > > > >> > > > > > unit > > > > >> > > > > > tested so that part is also essential. One key test here > > is > > > > when > > > > >> > > > > > certificates > > > > >> > > > > > are not changing then no action must be performed (not > to > > > > break > > > > >> the > > > > >> > > > whole > > > > >> > > > > > system apart). > > > > >> > > > > > > > > > >> > > > > > Purely personal opinion but such feature developments > are > > > slow > > > > >> by > > > > >> > > > nature > > > > >> > > > > > because > > > > >> > > > > > of edge case / stress testing. > > > > >> > > > > > > > > > >> > > > > > BR, > > > > >> > > > > > G > > > > >> > > > > > > > > > >> > > > > > > > > > >> > > > > > On Fri, Apr 18, 2025 at 4:53 PM Robert Metzger < > > > > >> > rmetz...@apache.org> > > > > >> > > > > > wrote: > > > > >> > > > > > > > > > >> > > > > > > Hi Nicolas, > > > > >> > > > > > > > > > > >> > > > > > > This looks like a nice improvement, thanks for the > write > > > up. > > > > >> > > > > > > Are you in touch with any committer who's willing to > > > review > > > > / > > > > >> > merge > > > > >> > > > > this? > > > > >> > > > > > > > > > > >> > > > > > > Some random questions on the FLIP: > > > > >> > > > > > > (1) "Each service that depends on TLS certificates > will > > > > >> > > initialize a > > > > >> > > > > > > FileSytemWatchService" > > > > >> > > > > > > > > > > >> > > > > > > It seems that there are 4 components using SSL, does > > this > > > > mean > > > > >> > > there > > > > >> > > > > will > > > > >> > > > > > > be 4 additional threads running, watching the same set > > of > > > > >> files? > > > > >> > > > > > > Wouldn't it be better to introduce a central file > > watching > > > > >> > service, > > > > >> > > > and > > > > >> > > > > > SSL > > > > >> > > > > > > users can subscribe to updates, to reduce the number > of > > > > >> threads? > > > > >> > > > > > > If this makes the whole effort 4x more complicated, I > > > > wouldn't > > > > >> > > > consider > > > > >> > > > > > it, > > > > >> > > > > > > but if its roughly the same effort, we should :) > > > > >> > > > > > > > > > > >> > > > > > > (2) "FileSytemWatchService" > > > > >> > > > > > > When I read this name, I was wondering, whether this > is > > > > >> somehow > > > > >> > > > related > > > > >> > > > > > to > > > > >> > > > > > > the Flink "FileSystem" classes. Which I think its' > not. > > > > >> > > > > > > Maybe a different name, that makes this separation > more > > > > >> explicit, > > > > >> > > > would > > > > >> > > > > > > make sense. Maybe "LocalFSWatchService"? > > > > >> > > > > > > (I'm sorry to bring up naming stuff -- its very > > > subjective, > > > > >> and > > > > >> > > > > > difficult) > > > > >> > > > > > > > > > > >> > > > > > > (3) For the test plan: There seem to be some SSL > related > > > e2e > > > > >> > tests: > > > > >> > > > > > > > > > > >> > > > > > > > > > > >> > > > > > > > > > >> > > > > > > > > >> > > > > > > > >> > > > > > > >> > > > > > >> > > > > > > > > > > https://github.com/apache/flink/blob/master/flink-end-to-end-tests/test-scripts/common_ssl.sh > > > > >> > > > > > > It would be nice to extend them to cover this feature > as > > > > >> well. I > > > > >> > > > would > > > > >> > > > > > hate > > > > >> > > > > > > for this feature to slowly break by future changes, so > > > good > > > > >> e2e > > > > >> > > test > > > > >> > > > > > > coverage is key, in particular bc so many components > are > > > > >> > involved. > > > > >> > > > > > > > > > > >> > > > > > > Best, > > > > >> > > > > > > Robert > > > > >> > > > > > > > > > > >> > > > > > > On Wed, Apr 16, 2025 at 11:55 AM Nicolas Fraison > > > > >> > > > > > > <nicolas.frai...@datadoghq.com.invalid> wrote: > > > > >> > > > > > > > > > > >> > > > > > > > Hi All, > > > > >> > > > > > > > > > > > >> > > > > > > > I'd like to start a discussion to Handle TLS > > Certificate > > > > >> > Renewal > > > > >> > > > > > > > Please provide some feedback on this proposal: > > > > >> > > > > > > > > > > > >> > > > > > > > > > > > >> > > > > > > > > > > >> > > > > > > > > > >> > > > > > > > > >> > > > > > > > >> > > > > > > >> > > > > > >> > > > > > > > > > > https://cwiki.apache.org/confluence/display/FLINK/FLIP-523%3A+Handle+TLS+Certificate+Renewal > > > > >> > > > > > > > > > > > >> > > > > > > > Regards, > > > > >> > > > > > > > > > > > >> > > > > > > > Nicolas Fraison > > > > >> > > > > > > > > > > > >> > > > > > > > > > > >> > > > > > > > > > >> > > > > > > > > >> > > > > > > > >> > > > > > > >> > > > > > >> > > > > > > > > > > > > > > >