Guys Thanks for your efforts to make it available in 2.4!
On Thu, Dec 28, 2017 at 4:15 PM, Andrey Gura <ag...@apache.org> wrote: > Guys, > > I've merged change to master branch. Thanks a lot! > > On Thu, Dec 28, 2017 at 3:54 PM, Petr Ivanov <mr.wei...@gmail.com> wrote: > > Fixed remarks and nitpicks. > > > > Also, I purpose to delay service autostart implementation until we’ll > 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 <ilya.kasnach...@gmail.com> > wrote: > >> > >> 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 > blame > >> - Ignite release process likely, not RPM building - but they shouldn't > be > >> here I guess? > >> > >> Regards! > >> > >> -- > >> Ilya Kasnacheev > >> > >> 2017-12-28 14:21 GMT+03:00 Ilya Kasnacheev <ilya.kasnach...@gmail.com>: > >> > >>> 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-ign...@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 > pages > >>> so people won't get lost. What do you all think? Should Ignite > auto-start > >>> 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 <mr.wei...@gmail.com>: > >>> > >>>> Removed replacement for default-config.xml. > >>>> Also reimplemented service to be able to run multiple instances of > Ignite. > >>>> > >>>> 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 <ptupit...@apache.org> > 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 <mr.wei...@gmail.com> > >>>> wrote: > >>>>> > >>>>>> Can we change default configuration file then? > >>>>>> So that both binary and package deliveries contained PDS turned on > by > >>>>>> default? > >>>>>> > >>>>>> And what about Multicast Discovery? > >>>>>> > >>>>>> > >>>>>>> On 25 Dec 2017, at 18:41, Dmitriy Setrakyan <dsetrak...@apache.org > > > >>>>>> 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 <ag...@apache.org> > >>>> 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 > use > >>>>>>>> default configuration file like to standard binary release. > >>>>>>>> > >>>>>>>> On Mon, Dec 25, 2017 at 4:39 PM, Ilya Kasnacheev > >>>>>>>> <ilya.kasnach...@gmail.com> wrote: > >>>>>>>>> Hello Igniters! > >>>>>>>>> > >>>>>>>>> What's your take on enabling PDS in 2.4 in default config? This > way > >>>> 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 <mr.wei...@gmail.com>: > >>>>>>>>> > >>>>>>>>>> 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 > that > >>>>>>>> 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 > over > >>>>>>>>>>> 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 > firewall > >>>>>> 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 > sqlline > >>>>>>>> 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.sh > >>>>>>>>>> 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 > package > >>>>>>>> build, > >>>>>>>>>> so that those executables work normally form the package > >>>> installation. > >>>>>>>>>> And lastly, redesign them to be universal and compatible with > all > >>>>>>>> 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 > guess > >>>>>>>> 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 > the > >>>>>>>> nearest > >>>>>>>>>> release our package built from binary archive and do not supply > >>>>>> sources > >>>>>>>>>> package (as it is currently done in apache.org/dist for > cassandra > >>>> / > >>>>>>>> hadoop > >>>>>>>>>> or alike). And later design package build procedure, which > includes > >>>>>>>> getting > >>>>>>>>>> sources from sources distribution, or even from Apache Ignite > Git > >>>>>>>>>> repository > >>>>>>>>>> tag. > >>>>>>>>>> > >>>>>>>>>> > >>>>>>>>>> Ilya Kasnacheev wrote > >>>>>>>>>>> spec contains version (2.4.0) - will be needed to be changed > for > >>>>>> 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 > it > >>>> 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 > Discovery > >>>>>>>> turned > >>>>>>>>>> on or not? > >>>>>>>>>> > >>>>>>>>>> Also I'll appreciate any other comments concerning current > package > >>>>>>>> design. > >>>>>>>>>> > >>>>>>>>>> > >>>>>>>>>> > >>>>>>>>>> -- > >>>>>>>>>> Sent from: http://apache-ignite-developers.2346864.n4.nabble. > com/ > >>>>>>>>>> > >>>>>>>> > >>>>>> > >>>>>> > >>>> > >>>> > >>> > > > -- Sergey Kozlov GridGain Systems www.gridgain.com
