Return-Path: X-Original-To: archive-asf-public-internal@cust-asf2.ponee.io Delivered-To: archive-asf-public-internal@cust-asf2.ponee.io Received: from cust-asf.ponee.io (cust-asf.ponee.io [163.172.22.183]) by cust-asf2.ponee.io (Postfix) with ESMTP id 7DC6E200D68 for ; Thu, 28 Dec 2017 14:15:22 +0100 (CET) Received: by cust-asf.ponee.io (Postfix) id 7C655160C1F; Thu, 28 Dec 2017 13:15:22 +0000 (UTC) Delivered-To: archive-asf-public@cust-asf.ponee.io Received: from mail.apache.org (hermes.apache.org [140.211.11.3]) by cust-asf.ponee.io (Postfix) with SMTP id 7415F160C16 for ; Thu, 28 Dec 2017 14:15:21 +0100 (CET) Received: (qmail 60830 invoked by uid 500); 28 Dec 2017 13:15:20 -0000 Mailing-List: contact dev-help@ignite.apache.org; run by ezmlm Precedence: bulk List-Help: List-Unsubscribe: List-Post: List-Id: Reply-To: dev@ignite.apache.org Delivered-To: mailing list dev@ignite.apache.org Received: (qmail 60819 invoked by uid 99); 28 Dec 2017 13:15:20 -0000 Received: from mail-relay.apache.org (HELO mail-relay.apache.org) (140.211.11.15) by apache.org (qpsmtpd/0.29) with ESMTP; Thu, 28 Dec 2017 13:15:20 +0000 Received: from mail-lf0-f46.google.com (mail-lf0-f46.google.com [209.85.215.46]) by mail-relay.apache.org (ASF Mail Server at mail-relay.apache.org) with ESMTPSA id CCDAA1A0094 for ; Thu, 28 Dec 2017 13:15:19 +0000 (UTC) Received: by mail-lf0-f46.google.com with SMTP id f13so45243751lff.12 for ; Thu, 28 Dec 2017 05:15:19 -0800 (PST) X-Gm-Message-State: AKGB3mI0m+zrx4HxIqa2cumdo1XMSRV/e7aA2HuUUxHEhykcKPihnUE+ gPcKqHpqZO/t2EKfHnsgtFgBzsikm0kVt3s3Oao= X-Google-Smtp-Source: ACJfBos24ZFWSxQGm6yBFpSmHYE/iy7KIo5rfW/IxFw8fPZ1t3BLc6691L/wcrEllr60it3+H2cCYUP0VMm59pg3VQE= X-Received: by 10.25.154.196 with SMTP id c187mr4883104lfe.116.1514466917277; Thu, 28 Dec 2017 05:15:17 -0800 (PST) MIME-Version: 1.0 Received: by 10.25.19.169 with HTTP; Thu, 28 Dec 2017 05:15:16 -0800 (PST) In-Reply-To: References: <247BA016-11D2-4ADA-9E76-692D99010ABB@gmail.com> <1513236833514-0.post@n4.nabble.com> <863C79DE-4539-40C7-B28E-7CC9D0F677D2@apache.org> <1513583946539-0.post@n4.nabble.com> <4F0BD90E-9CC5-47C7-95D7-377F59BAE9C4@apache.org> <1513679026288-0.post@n4.nabble.com> <1513956471964-0.post@n4.nabble.com> <3B61186C-DFA5-4CF8-AE91-843C050E2717@gmail.com> <5B46309D-1504-4142-A830-B5F18F5D02A0@gmail.com> From: Andrey Gura Date: Thu, 28 Dec 2017 16:15:16 +0300 X-Gmail-Original-Message-ID: Message-ID: Subject: Re: IGNITE-7107 Apache Ignite RPM packages To: dev@ignite.apache.org Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable archived-at: Thu, 28 Dec 2017 13:15:22 -0000 Guys, I've merged change to master branch. Thanks a lot! On Thu, Dec 28, 2017 at 3:54 PM, Petr Ivanov wrote: > Fixed remarks and nitpicks. > > Also, I purpose to delay service autostart implementation until we=E2=80= =99ll get some first-hand usability feedback. > Added notes to DEVNOTES.txt about how to start service. > > >> On 28 Dec 2017, at 14:32, Ilya Kasnacheev wr= ote: >> >> Hello once more! >> >> Two more nitpicks: >> >> right now we install /usr/bin/ignitevisorcmd alternative, but it does't >> work since a) visor wants to write to /usr/share, and b) we moved it to >> examples for that. >> Please remove that alternative for now, create a ticket. >> >> rpm -ql apache-ignite | grep \\.java >> some yardstick and ml sources are there. I don't know who is there to bl= ame >> - Ignite release process likely, not RPM building - but they shouldn't b= e >> here I guess? >> >> Regards! >> >> -- >> Ilya Kasnacheev >> >> 2017-12-28 14:21 GMT+03:00 Ilya Kasnacheev : >> >>> Hello again! >>> >>> I have reviewed the modified patch. All the things in my critical list >>> were fixed. I have just two minor remarks: >>> >>> - Please move sqlline.sh back from examples to /usr/share/a-i/bin. It >>> works just as well from there as it was from our distribution. visor >>> doesn't, hence it stays in examples. >>> - I'm a bit confused that we no longer auto-start service after package= is >>> installed. You have to do >>> sudo systemctl start apache-ignite@default-config.xml >>> manually. I'm used to packages that start the thing they install >>> immediately. Of course we can just document that clearly on downloads p= ages >>> so people won't get lost. What do you all think? Should Ignite auto-sta= rt >>> with default config? Is it possible to do so, but make possible to turn= it >>> off. >>> >>> So from my side I recommend this pull request for inclusion after 1) is >>> done and 2) is discussed. >>> >>> Also, would be nice if you create additional JIRA tickets for issues in= my >>> minor list (ignitesqlline in /usr/bin, ignitevisorcmd in /usr/bin and >>> working) to be implemented in future releases. >>> >>> Regards, >>> >>> -- >>> Ilya Kasnacheev >>> >>> 2017-12-26 15:25 GMT+03:00 Petr Ivanov : >>> >>>> Removed replacement for default-config.xml. >>>> Also reimplemented service to be able to run multiple instances of Ign= ite. >>>> >>>> See updated PR [1]. >>>> >>>> >>>> [1] https://github.com/apache/ignite/pull/3171 < >>>> https://github.com/apache/ignite/pull/3171> >>>> >>>> >>>> >>>>> On 25 Dec 2017, at 18:57, Pavel Tupitsyn wrote= : >>>>> >>>>> PDS and discovery settings are unrelated to the packaging task. >>>>> Andrey is right, just use existing default config. >>>>> >>>>> On Mon, Dec 25, 2017 at 6:51 PM, Petr Ivanov >>>> wrote: >>>>> >>>>>> Can we change default configuration file then? >>>>>> So that both binary and package deliveries contained PDS turned on b= y >>>>>> default? >>>>>> >>>>>> And what about Multicast Discovery? >>>>>> >>>>>> >>>>>>> On 25 Dec 2017, at 18:41, Dmitriy Setrakyan >>>>>> wrote: >>>>>>> >>>>>>> I agree with Andrey. The product should be identical regardless of = how >>>>>> you >>>>>>> download and install it. >>>>>>> >>>>>>> On Mon, Dec 25, 2017 at 7:18 AM, Andrey Gura >>>> wrote: >>>>>>> >>>>>>>> Hi, >>>>>>>> >>>>>>>> I think we should provide the same default configuration for all >>>>>>>> binary builds. From my point of view RPM/DEB/etc packages should u= se >>>>>>>> default configuration file like to standard binary release. >>>>>>>> >>>>>>>> On Mon, Dec 25, 2017 at 4:39 PM, Ilya Kasnacheev >>>>>>>> wrote: >>>>>>>>> Hello Igniters! >>>>>>>>> >>>>>>>>> What's your take on enabling PDS in 2.4 in default config? This w= ay >>>> we >>>>>>>>> could enable it in RPM build with easy heart. >>>>>>>>> >>>>>>>>> The rest of your answers seem spot on. I'll happily review your >>>> amended >>>>>>>>> patch when it is ready (later this week?) >>>>>>>>> >>>>>>>>> -- >>>>>>>>> Ilya Kasnacheev >>>>>>>>> >>>>>>>>> 2017-12-22 18:27 GMT+03:00 vveider : >>>>>>>>> >>>>>>>>>> Ilya Kasnacheev wrote >>>>>>>>>>> I have noticed that both spec and DEVNOTES are made to use tabs >>>>>>>> alongside >>>>>>>>>>> with spaces. I thought we are avoiding tabs? Please confirm tha= t >>>>>>>> usage of >>>>>>>>>>> tabs is desired in this case. >>>>>>>>>> >>>>>>>>>> My fault - got used to editing such files in default vim. Fixed = and >>>>>>>> updated >>>>>>>>>> vim settings to indent with spaces. >>>>>>>>>> >>>>>>>>>> >>>>>>>>>> Ilya Kasnacheev wrote >>>>>>>>>>> Maybe it's my CentOS but initially build will fail with the >>>> following >>>>>>>>>>> message, which I had to fix in spec >>>>>>>>>>> + cd 'apache-ignite-*' >>>>>>>>>>> /var/tmp/rpm-tmp.KwOoyl: line 34: cd: apache-ignite-*: No such >>>> file >>>>>> or >>>>>>>>>>> directory >>>>>>>>>> >>>>>>>>>> Strange error - shell expansion is not working. Anyway, replaced >>>> with >>>>>>>> more >>>>>>>>>> universal variant. >>>>>>>>>> >>>>>>>>>> >>>>>>>>>> Ilya Kasnacheev wrote >>>>>>>>>>> We absolutely should not inline configuration files (such as >>>>>>>>>>> default-config.xml) into build scripts. We should just copy ove= r >>>>>>>>>>> default-config.xml from config/ to /etc, with dependencies if >>>> needed. >>>>>>>>>> >>>>>>>>>> Extracted corresponding sections into separate files. >>>>>>>>>> >>>>>>>>>> >>>>>>>>>> Ilya Kasnacheev wrote >>>>>>>>>>> I'm not sure PDS should be ON by default. Better provide it in >>>>>>>> examples >>>>>>>>>>> but disable by default. >>>>>>>>>> >>>>>>>>>> AFAIK, PDS is actively pushing forward and has to be enabled by >>>>>> default >>>>>>>> so >>>>>>>>>> that Ignite users start working with PDS from the very beginning= . >>>>>>>>>> >>>>>>>>>> >>>>>>>>>> Ilya Kasnacheev wrote >>>>>>>>>>> I cannot comment on firewall rules validity, but firewall and >>>> service >>>>>>>>>>> configurations should be stored in sources, as files, supplied >>>> with >>>>>>>>>>> release (somewhere in packages/) and installed from there >>>>>>>>>> >>>>>>>>>> Unfortunately, I did not find any other way to open ports and >>>>>> multicast, >>>>>>>>>> except applying direct iptables rules though firewall-cmd -- so >>>> there >>>>>>>> can >>>>>>>>>> be >>>>>>>>>> no separate service firewall rules file (as can be found here: >>>>>>>>>> /usr/lib/firewalld/services). Applied rules from package go >>>> straight >>>>>> to >>>>>>>>>> /etc/firewalld/direct.xml. If we agree not to put >>>> Multicast-enabled by >>>>>>>>>> default configuration file and will stick to IP Discovery, I'll >>>> try to >>>>>>>>>> revise firewall configuration and create separate service firewa= ll >>>>>> rules >>>>>>>>>> file (but, I guess, much later). >>>>>>>>>> >>>>>>>>>> >>>>>>>>>> Ilya Kasnacheev wrote >>>>>>>>>>> * sqlline.sh fails to connect initially, needs to be specified >>>>>>>>>>> jdbc:ignite:thin://localhost, then it works. I think that sqlli= ne >>>>>>>> should >>>>>>>>>>> become available as /usr/bin/apache-ignite-sqlline for example, >>>> to be >>>>>>>> in >>>>>>>>>>> $PATH. And figure out how to connect by itself. >>>>>>>>>>> * ignitevisorcmd.sh becomes confused. first it scans >>>>>>>>>>> /usr/share/apache-ignite for configs, then it fails to write to >>>>>>>>>>> /usr/share/apache-ignite/work. We should consider how it can be >>>> fixed >>>>>>>> (a >>>>>>>>>>> symlink from /usr/share/apache-ignite/config to >>>> /etc/apache-ignite, >>>>>>>>>>> perhaps, and work dir in /tmp?), and made available as >>>>>>>>>>> /usr/bin/apache-ignite-visorcmd. >>>>>>>>>> >>>>>>>>>> The problem exists mainly because of sqlline.sh|ignitevisorcmd.s= h >>>>>>>>>> unaware-of-package design and I see the following 3-step way to >>>>>> overcome >>>>>>>>>> this limitation. >>>>>>>>>> At first we hide this executables file in examples (As Iliya >>>>>> proposed). >>>>>>>>>> Secondly, I can design a patch, which can be applied during pack= age >>>>>>>> build, >>>>>>>>>> so that those executables work normally form the package >>>> installation. >>>>>>>>>> And lastly, redesign them to be universal and compatible with al= l >>>>>>>> delivery >>>>>>>>>> methods. >>>>>>>>>> >>>>>>>>>> >>>>>>>>>> Ilya Kasnacheev wrote >>>>>>>>>>> Not everything should go into /usr/share. classpath libs belong= to >>>>>>>>>>> /usr/lib. >>>>>>>>>> >>>>>>>>>> Moved libs to /usr/lib/apache-ignite and mapped to >>>>>>>>>> /usr/share/apache-ignite/libs (for backward compatibility). I gu= ess >>>>>>>> later >>>>>>>>>> we >>>>>>>>>> have to think out of a solution to load libs directly from /usr/= lib >>>>>>>> (along >>>>>>>>>> with configurations / logs / work / etc.) >>>>>>>>>> >>>>>>>>>> >>>>>>>>>> Ilya Kasnacheev wrote >>>>>>>>>>> We are building from binary package and not from sources. If it= is >>>>>>>> used >>>>>>>>>> by >>>>>>>>>>> internal RPM building this is tolerable, but any distro or >>>> repository >>>>>>>>>> will >>>>>>>>>>> want to build from source. >>>>>>>>>> >>>>>>>>>> It's not an ordinary task to create "100% honest" package, so I >>>> guess >>>>>> we >>>>>>>>>> definitely won't make to 2.4 release. So I purpose include in th= e >>>>>>>> nearest >>>>>>>>>> release our package built from binary archive and do not supply >>>>>> sources >>>>>>>>>> package (as it is currently done in apache.org/dist for cassandr= a >>>> / >>>>>>>> hadoop >>>>>>>>>> or alike). And later design package build procedure, which inclu= des >>>>>>>> getting >>>>>>>>>> sources from sources distribution, or even from Apache Ignite Gi= t >>>>>>>>>> repository >>>>>>>>>> tag. >>>>>>>>>> >>>>>>>>>> >>>>>>>>>> Ilya Kasnacheev wrote >>>>>>>>>>> spec contains version (2.4.0) - will be needed to be changed fo= r >>>>>> every >>>>>>>>>>> release. I'm not sure if it's possible to extract. >>>>>>>>>> >>>>>>>>>> Release procedure on TC is already has some task for project >>>> version >>>>>>>>>> update, >>>>>>>>>> so I guess it is not so difficult to update it accordingly (as i= t >>>> is >>>>>>>>>> currently done for C++ / .Net internal versions). >>>>>>>>>> >>>>>>>>>> >>>>>>>>>> Ilya Kasnacheev wrote >>>>>>>>>>> Package description may be improved. service description lacks >>>> '-' in >>>>>>>> "In >>>>>>>>>>> Memory". >>>>>>>>>> >>>>>>>>>> Fixed. >>>>>>>>>> >>>>>>>>>> >>>>>>>>>> So the only question to discuss (according Iliya's review) is >>>> whether >>>>>> to >>>>>>>>>> supply package with default-config.xml that has Multicast Discov= ery >>>>>>>> turned >>>>>>>>>> on or not? >>>>>>>>>> >>>>>>>>>> Also I'll appreciate any other comments concerning current packa= ge >>>>>>>> design. >>>>>>>>>> >>>>>>>>>> >>>>>>>>>> >>>>>>>>>> -- >>>>>>>>>> Sent from: http://apache-ignite-developers.2346864.n4.nabble.com= / >>>>>>>>>> >>>>>>>> >>>>>> >>>>>> >>>> >>>> >>> >