ignite-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Andrey Gura <ag...@apache.org>
Subject Re: IGNITE-7107 Apache Ignite RPM packages
Date Thu, 28 Dec 2017 13:15:16 GMT
Guys,

I've merged change to master branch. Thanks a lot!

On Thu, Dec 28, 2017 at 3:54 PM, Petr Ivanov <mr.weider@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.kasnacheev@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.kasnacheev@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-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 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.weider@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 <ptupitsyn@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.weider@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 <dsetrakyan@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 <agura@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.kasnacheev@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.weider@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/
>>>>>>>>>>
>>>>>>>>
>>>>>>
>>>>>>
>>>>
>>>>
>>>
>

Mime
View raw message