ignite-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Sergey Kozlov <skoz...@gridgain.com>
Subject Re: IGNITE-7107 Apache Ignite RPM packages
Date Thu, 28 Dec 2017 13:23:58 GMT
Guys

Thanks for your efforts to make it available in 2.4!

On Thu, Dec 28, 2017 at 4:15 PM, Andrey Gura <agura@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.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/
> >>>>>>>>>>
> >>>>>>>>
> >>>>>>
> >>>>>>
> >>>>
> >>>>
> >>>
> >
>



-- 
Sergey Kozlov
GridGain Systems
www.gridgain.com

Mime
  • Unnamed multipart/alternative (inline, None, 0 bytes)
View raw message