ignite-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Ilya Kasnacheev <ilya.kasnach...@gmail.com>
Subject Re: IGNITE-7107 Apache Ignite RPM packages
Date Thu, 28 Dec 2017 11:32:08 GMT
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
  • Unnamed multipart/alternative (inline, None, 0 bytes)
View raw message