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:21:09 GMT
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