ignite-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Anton Vinogradov ...@apache.org>
Subject Re: Consistency check and fix (review request)
Date Tue, 16 Apr 2019 11:20:34 GMT
Nikolay, that was the first approach

>> I think we should allow to the administrator to enable/disable
Consistency check.
In that case, we have to introduce cluster-wide change-strategy operation,
since every client node should be aware of the change.
Also, we have to specify caches list, and for each - should we check each
request or only 5-th and so on.
Procedure and configuration become overcomplicated in this case.

My idea that specific service will be able to use a special proxy according
to its own strategy
(eg. when administrator inside the building and boss is sleeping - all
operations on "cache[a,b,c]ed*" should check the consistency).
All service clients will have the same guarantees in that case.

So in other words, consistency should be guaranteed by service, not by
Ignite.
Service should guarantee consistency not only using new proxy but, for
example, using correct isolation fo txs.
That's not a good Idea to specify isolation mode for Ignite, same situation
with get-with-consistency-check.

On Tue, Apr 16, 2019 at 12:56 PM Nikolay Izhikov <nizhikov@apache.org>
wrote:

> Hello, Anton.
>
> > Customer should be able to change strategy on the fly according to time>
> periods or load.
>
> I think we should allow to administrator to enable/disable Consistency
> check.
> This option shouldn't be related to application code because "Consistency
> check" is some kind of maintance procedure.
>
> What do you think?
>
> В Вт, 16/04/2019 в 12:47 +0300, Anton Vinogradov пишет:
> > Andrey, thanks for tips
> >
> > > > You can perform consistency check using idle verify utility.
> >
> > Could you please point to utility's page?
> > According to its name, it requires to stop the cluster to perform the
> check?
> > That's impossible at real production when you should have downtime less
> > that some minutes per year.
> > So, the only case I see is to use online check during periods of moderate
> > activity.
> >
> > > > Recovery tool is good idea
> >
> > This tool is a part of my IEP.
> > But recovery tool (process)
> > - will allow you to check entries in memory only (otherwise, you will
> warm
> > up the cluster incorrectly), and that's a problem when you have
> > persisted/in_memory rate > 10:1
> > - will cause latency drop for some (eg. 90+ percentile) requests, which
> is
> > not acceptable for real production, when we have strict SLA.
> > - will not guarantee that each operation will use consistent data,
> > sometimes it's extremely essential
> > so, the process is a cool idea, but, sometime you may need more.
> >
> > Ivan, thanks for analysis
> >
> > > > why it comes as an on-demand enabled proxy but not as a mode enabled
> by
> >
> > some configuration property
> > It's a bad idea to have this feature permanently enabled, it slows down
> the
> > system by design.
> > Customer should be able to change strategy on the fly according to time
> > periods or load.
> > Also, we're going to use this proxy for odd requests or for every 5-th,
> > 10-th, 100-th request depends on the load/time/SLA/etc.
> > The goal is to perform as much as possible gets-with-consistency
> operations
> > without stopping the cluster and never find a problem :)
> >
> > > > As for me it will be great if we can improve consistency guarantees
> >
> > provided by default.
> > Once you checked backups you decreased throughput and increased latency.
> > This feature requred only for some financial, nuclear, health systems
> when
> > you should be additionally sure about consistency.
> > It's like a
> > - read from backups
> > - data modification outside the transaction
> > - using FULL_ASYNC instead of FULL_SYNC,
> > sometimes it's possible, sometimes not.
> >
> > > > 1. It sounds suspicious that reads can cause writes (unexpected
> >
> > deadlocks might be possible).
> > Code performs writes
> > - key per additional transaction in case original tx was OPTIMISTIC ||
> > READ_COMMITTED,
> > - all keys per same tx in case original tx was PESSIMISTIC &&
> > !READ_COMMITTED, since you already obtain the locks,
> > so, deadlock should be impossible.
> >
> > > > 2. I do not believe that it is possible to implement a (bugless?)
> >
> > feature which will fix other bugs.
> > It does not fix the bugs, it looks for inconsistency (no matter how it
> > happened) and reports using events (previous state and how it was fixed).
> > This allows continuing processing for all the entries, even inconsistent.
> > But, each such fix should be rechecked manually, for sure.
> >
> > On Tue, Apr 16, 2019 at 11:39 AM Павлухин Иван <vololo100@gmail.com>
> wrote:
> >
> > > Anton,
> > >
> > > Thank you for your effort for improving consistency guarantees
> > > provided by Ignite.
> > >
> > > The subject sounds really vital. Could you please elaborate why it
> > > comes as an on-demand enabled proxy but not as a mode enabled by some
> > > configuration property (or even as a default behavior)? How do you see
> > > the future development of such consistency checks? As for me it will
> > > be great if we can improve consistency guarantees provided by default.
> > >
> > > Also thinking loud a bit:
> > > 1. It sounds suspicious that reads can cause writes (unexpected
> > > deadlocks might be possible).
> > > 2. I do not believe that it is possible to implement a (bugless?)
> > > feature which will fix other bugs.
> > > 3. A storage (or database) product (Ignite in our case) consistency is
> > > not equal to a user application consistency. So, it might be that
> > > introduced checks are insufficient to make business applications
> > > happy.
> > >
> > > пн, 15 апр. 2019 г. в 19:27, Andrey Gura <agura@apache.org>:
> > > >
> > > > Anton,
> > > >
> > > > I'm trying tell you that this proxy can produce false positive
> result,
> > > > incorrect result and just hide bugs. What will the next solution?
> > > > withNoBugs proxy?
> > > >
> > > > You can perform consistency check using idle verify utility. Recovery
> > > > tool is good idea but user should trigger this process, not some
> cache
> > > > proxy implementation.
> > > >
> > > > On Mon, Apr 15, 2019 at 5:34 PM Anton Vinogradov <av@apache.org>
> wrote:
> > > > >
> > > > > Seems, we already fixed all bugs caused this feature, but there is
> no
> > > > > warranty we will not create new :)
> > > > > This proxy is just checker that consistency is ok.
> > > > >
> > > > > > > reaching bugless implementation
> > > > >
> > > > > Not sure it's possible. Once you have software it contains bugs.
> > > > > This proxy will tell you whether these bugs lead to inconsistency.
> > > > >
> > > > > On Mon, Apr 15, 2019 at 5:19 PM Andrey Gura <agura@apache.org>
> wrote:
> > > > >
> > > > > > Method name is minor problem. I still believe that there is
no
> need
> > > > > > for this proxy because there are no any guarantees about bugless
> > > > > > implementation this functionality. Better way is reaching bugless
> > > > > > implementation of current functionality.
> > > > > >
> > > > > > On Mon, Apr 15, 2019 at 4:51 PM Anton Vinogradov <av@apache.org>
> > >
> > > wrote:
> > > > > > >
> > > > > > > Andrey,
> > > > > > >
> > > > > > > > > It means also that at least method name is bad.
> > > > > > >
> > > > > > > Agreed, already discussed with Aleksey Plekhanov.
> > > > > > > Decided that ".withConsistencyCheck()" is a proper name.
> > > > > > >
> > > > > > > > > What is the profit?
> > > > > > >
> > > > > > > This proxy allows to check (and fix) is there any consistency
> > >
> > > violation
> > > > > > > across the topology.
> > > > > > > The proxy will check all backups contain the same values
as
> > >
> > > primary.
> > > > > > > So, when it's possible (you're ready to spend resources
for
> this
> > >
> > > check)
> > > > > > you
> > > > > > > will be able to read-with-consistency-check.
> > > > > > > This will decrease the amount of "inconsistency caused
> > > > > > > war/strikes/devastation" situations, which is important
for
> > >
> > > financial
> > > > > > > systems.
> > > > > > >
> > > > > > > On Mon, Apr 15, 2019 at 3:58 PM Andrey Gura <agura@apache.org>
> > >
> > > wrote:
> > > > > > >
> > > > > > > > Anton,
> > > > > > > >
> > > > > > > > what does expression "withConsistency" mean? From
user's
> > >
> > > standpoint it
> > > > > > > > means that all operations performed without this proxy
are
> not
> > > > > > > > consistent. It means also that at least method name
is bad.
> > > > > > > >
> > > > > > > > Are there any guarantees that withConsistency proxy
will not
> > >
> > > contain
> > > > > > > > bugs that will lead to inconsistent write after
> inconsistency was
> > > > > > > > found? I think there are no such guarantees. Bugs
still are
> > >
> > > possible.
> > > > > > > > So I always must use withConsistency proxy because
I doesn't
> have
> > > > > > > > other choice - all ways are unreliable and withConsistency
> just
> > >
> > > sounds
> > > > > > > > better.
> > > > > > > >
> > > > > > > > Eventually we will have two different ways for working
with
> cache
> > > > > > > > values with different bugs set. What is the profit?
> > > > > > > >
> > > > > > > >
> > > > > > > >
> > > > > > > > On Fri, Apr 12, 2019 at 2:49 PM Anton Vinogradov <
> av@apache.org>
> > > > > >
> > > > > > wrote:
> > > > > > > > >
> > > > > > > > > Folks,
> > > > > > > > >
> > > > > > > > > I've checked the tx benchmarks and found no performance
> drop.
> > > > > > > > > Also, see no issues at TC results.
> > > > > > > > > So, seems, code ready to be merged.
> > > > > > > > >
> > > > > > > > > Everyone interested, please share any objections
about
> > > > > > > > > - public API
> > > > > > > > > - test coverage
> > > > > > > > > - implementation approach
> > > > > > > > >
> > > > > > > > > On Wed, Apr 3, 2019 at 5:46 PM Anton Vinogradov
<
> av@apache.org
> > > > > > wrote:
> > > > > > > > >
> > > > > > > > > > Nikolay,
> > > > > > > > > >
> > > > > > > > > > This is not a PoC, but the final solution
(I hope so:) )
> > >
> > > required
> > > > > > the
> > > > > > > > > > review.
> > > > > > > > > > LWW means Last Write Wins, detailed explanation
can be
> found
> > >
> > > at
> > > > > > IEP-31.
> > > > > > > > > >
> > > > > > > > > > On Wed, Apr 3, 2019 at 5:24 PM Nikolay Izhikov
<
> > > > > >
> > > > > > nizhikov@apache.org>
> > > > > > > > > > wrote:
> > > > > > > > > >
> > > > > > > > > > > Hello, Anton.
> > > > > > > > > > >
> > > > > > > > > > > Thanks for the PoC.
> > > > > > > > > > >
> > > > > > > > > > > > finds correct values according
to LWW strategy
> > > > > > > > > > >
> > > > > > > > > > > Can you, please, clarify what is LWW
strategy?
> > > > > > > > > > >
> > > > > > > > > > > В Ср, 03/04/2019 в 17:19 +0300,
Anton Vinogradov пишет:
> > > > > > > > > > > > Ilya,
> > > > > > > > > > > >
> > > > > > > > > > > > This is impossible due to a conflict
between some
> > >
> > > isolation
> > > > > > levels
> > > > > > > > and
> > > > > > > > > > > > get-with-consistency expectations.
> > > > > > > > > > > > Basically, it's impossible to
perform
> get-with-consistency
> > > > > >
> > > > > > after the
> > > > > > > > > > > other
> > > > > > > > > > > > get at !READ_COMMITTED transaction.
> > > > > > > > > > > > The problem here is that value
should be cached
> according
> > >
> > > to the
> > > > > > > > > > > isolation
> > > > > > > > > > > > level, so get-with-consistency
is restricted in this
> case.
> > > > > > > > > > > > Same problem we have at case get-with-consistency
> after
> > >
> > > put, so
> > > > > > we
> > > > > > > > have
> > > > > > > > > > > > restriction here too.
> > > > > > > > > > > > So, the order matter. :)
> > > > > > > > > > > >
> > > > > > > > > > > > See OperationRestrictionsCacheConsistencyTest
[1] for
> > >
> > > details.
> > > > > > > > > > > >
> > > > > > > > > > > > [1]
> > > > > > > > > > > >
> > >
> > >
> https://github.com/apache/ignite/blob/8b0b0c3e1bde93ff9c4eb5667d794dd64a8b06f0/modules/core/src/test/java/org/apache/ignite/internal/processors/cache/consistency/OperationRestrictionsCacheConsistencyTest.java
> > > > > > > > > > > >
> > > > > > > > > > > > On Wed, Apr 3, 2019 at 4:54 PM
Ilya Kasnacheev <
> > > > > > > > > > >
> > > > > > > > > > > ilya.kasnacheev@gmail.com>
> > > > > > > > > > > > wrote:
> > > > > > > > > > > >
> > > > > > > > > > > > > Hello!
> > > > > > > > > > > > >
> > > > > > > > > > > > > Sounds useful especially
for new feature
> development.
> > > > > > > > > > > > >
> > > > > > > > > > > > > Can you do a run of all tests
with
> > >
> > > cache.forConsistency(),
> > > > > > see if
> > > > > > > > > > > there are
> > > > > > > > > > > > > cases that fail?
> > > > > > > > > > > > >
> > > > > > > > > > > > > Regards,
> > > > > > > > > > > > > --
> > > > > > > > > > > > > Ilya Kasnacheev
> > > > > > > > > > > > >
> > > > > > > > > > > > >
> > > > > > > > > > > > > ср, 3 апр. 2019 г.
в 16:17, Anton Vinogradov <
> > >
> > > av@apache.org>:
> > > > > > > > > > > > >
> > > > > > > > > > > > > > Igniters,
> > > > > > > > > > > > > >
> > > > > > > > > > > > > > Sometimes, at real deployment,
we're faced with
> > >
> > > inconsistent
> > > > > > > > state
> > > > > > > > > > > across
> > > > > > > > > > > > > > the topology.
> > > > > > > > > > > > > > This means that somehow
we have different values
> for
> > >
> > > the
> > > > > > same
> > > > > > > > key at
> > > > > > > > > > > > > > different nodes.
> > > > > > > > > > > > > > This is an extremely
rare situation, but, when
> you
> > >
> > > have
> > > > > > > > thousands of
> > > > > > > > > > > > > > terabytes of data, this
can be a real problem.
> > > > > > > > > > > > > >
> > > > > > > > > > > > > > Apache Ignite provides
a consistency guarantee,
> each
> > > > > >
> > > > > > affinity
> > > > > > > > node
> > > > > > > > > > > should
> > > > > > > > > > > > > > contain the same value
for the same key, at least
> > > > > >
> > > > > > eventually.
> > > > > > > > > > > > > > But this guarantee can
be violated because of
> bugs,
> > >
> > > see
> > > > > > IEP-31
> > > > > > > > [1]
> > > > > > > > > > > for
> > > > > > > > > > > > > > details.
> > > > > > > > > > > > > >
> > > > > > > > > > > > > > So, I created the issue
[2] to handle such
> situations.
> > > > > > > > > > > > > > The main idea is to
have a special
> > >
> > > cache.withConsistency()
> > > > > > proxy
> > > > > > > > > > > allows
> > > > > > > > > > > > > > checking a fix inconsistency
on get operation.
> > > > > > > > > > > > > >
> > > > > > > > > > > > > > I've created PR [3]
with following improvements
> (when
> > > > > > > > > > > > > > cache.withConsistency()
proxy used):
> > > > > > > > > > > > > >
> > > > > > > > > > > > > > - PESSIMISTIC &&
!READ_COMMITTED transaction
> > > > > > > > > > > > > > -- checks values across
the topology (under
> locks),
> > > > > > > > > > > > > > -- finds correct values
according to LWW
> strategy,
> > > > > > > > > > > > > > -- records special event
in case consistency
> > >
> > > violation found
> > > > > > > > > > > (contains
> > > > > > > > > > > > > > inconsistent map <Node,
<K,V>> and last values
> <K,V>),
> > > > > > > > > > > > > > -- enlists writes with
latest value for each
> > >
> > > inconsistent
> > > > > > key,
> > > > > > > > so
> > > > > > > > > > > it will
> > > > > > > > > > > > > > be written on tx.commit().
> > > > > > > > > > > > > >
> > > > > > > > > > > > > > - OPTIMISTIC || READ_COMMITTED
transactions
> > > > > > > > > > > > > > -- checks values across
the topology (not under
> > >
> > > locks, so
> > > > > > > > > > > false-positive
> > > > > > > > > > > > > > case is possible),
> > > > > > > > > > > > > > -- starts PESSIMISTIC
&& SERIALIZABLE (at
> separate
> > >
> > > thread)
> > > > > > > > > > > transaction
> > > > > > > > > > > > >
> > > > > > > > > > > > > for
> > > > > > > > > > > > > > each possibly broken
key and fixes it on a
> commit if
> > > > > >
> > > > > > necessary.
> > > > > > > > > > > > > > -- original transaction
performs get-after-fix
> and
> > >
> > > can be
> > > > > > > > continued
> > > > > > > > > > > if
> > > > > > > > > > > > >
> > > > > > > > > > > > > the
> > > > > > > > > > > > > > fix does not conflict
with isolation level.
> > > > > > > > > > > > > >
> > > > > > > > > > > > > > Future plans
> > > > > > > > > > > > > > - Consistency guard
(special process periodically
> > >
> > > checks we
> > > > > > > > have no
> > > > > > > > > > > > > > inconsistency).
> > > > > > > > > > > > > > - MVCC support.
> > > > > > > > > > > > > > - Atomic caches support.
> > > > > > > > > > > > > > - Thin client support.
> > > > > > > > > > > > > > - SQL support.
> > > > > > > > > > > > > > - Read-with-consistency
before the write
> operation.
> > > > > > > > > > > > > > - Single key read-with-consistency
optimization,
> now
> > >
> > > the
> > > > > > > > collection
> > > > > > > > > > > > > > approach used each time.
> > > > > > > > > > > > > > - Do not perform read-with-consistency
for the
> key in
> > >
> > > case
> > > > > > it
> > > > > > > > was
> > > > > > > > > > > > > > consistently read some
time ago.
> > > > > > > > > > > > > >
> > > > > > > > > > > > > > [1]
> > > > > > > > > > > > > >
> > > > > > > > > > > > > >
> > > > > > > > > > > > >
> > > > > > > > > > > > >
> > >
> > >
> https://cwiki.apache.org/confluence/display/IGNITE/IEP-31+Consistency+check+and+fix
> > > > > > > > > > > > > > [2]
> > >
> > > https://issues.apache.org/jira/browse/IGNITE-10663
> > > > > > > > > > > > > > [3] https://github.com/apache/ignite/pull/5656
> > > > > > > > > > > > > >
> > >
> > >
> > >
> > > --
> > > Best regards,
> > > Ivan Pavlukhin
> > >
>

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