ScriptedIndexResult patch converts all .0 decimals to integers

38 views
Skip to first unread message

Gareth Thackeray

unread,
Oct 21, 2014, 11:50:17 AM10/21/14
to rav...@googlegroups.com
Hi,

If you have a document with a decimal value in it with decimal value (e.g. 5.0), and this document is updated by the ScriptedIndexResults bundle, then the patcher converts that decimal to an integer.

This then means that the next time the client loads the patched document it will save it back to the database, converting the integers back to the decimals.

The ScriptedJsonPatcher actually explicitly states it's going to do this: https://github.com/ravendb/ravendb/blob/master/Raven.Database/Json/ScriptedJsonPatcher.cs#L216

I can see why it would do this (because JS does not distinguish) but it was / is causing quite a problem for us.  I can get round it by evicting the patched document at certain times, but as a general principle the patcher updating fields it's not supposed to be concerned with seems wrong.

Cheers,

Gaz

Kijana Woodard

unread,
Oct 21, 2014, 12:25:18 PM10/21/14
to rav...@googlegroups.com
What sort of problems was it causing?

I was worrying about something similar the other day, not in a ravendb context, and convinced myself it wouldn't be an issue. :-/

--
You received this message because you are subscribed to the Google Groups "RavenDB - 2nd generation document database" group.
To unsubscribe from this group and stop receiving emails from it, send an email to ravendb+u...@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.

Gareth Thackeray

unread,
Oct 21, 2014, 12:40:24 PM10/21/14
to <ravendb@googlegroups.com>
We have the following:

web request starts 
  -> loads document A 
  -> updates document B in separate session 
  -> B updates A through a scripted index results patch 
  -> web request saves A because it was updated the last time this happened and decimals got converted to ints
  -> FAIL, because A got updated by the separate session

What I'm doing is totally unorthodox and when I've finished I'm hoping to post the details in here for the group's edification.  I think it'll make an interesting story whether it works or not!


--
You received this message because you are subscribed to a topic in the Google Groups "RavenDB - 2nd generation document database" group.
To unsubscribe from this topic, visit https://groups.google.com/d/topic/ravendb/qXD_U-S4aTY/unsubscribe.
To unsubscribe from this group and all its topics, send an email to ravendb+u...@googlegroups.com.

For more options, visit https://groups.google.com/d/optout.



--
Gareth Thackeray

Co-founder

Code Trip
Online Marketplaces




Kijana Woodard

unread,
Oct 21, 2014, 12:43:15 PM10/21/14
to rav...@googlegroups.com
Oh I see. It's a meaningless "change" that ends up conflicting the SIR request. Is that correct?

Gareth Thackeray

unread,
Oct 21, 2014, 12:50:39 PM10/21/14
to <ravendb@googlegroups.com>
Yes - the only "change" is converting "integers" back to decimals in the JSON.  But this fails because SIR has updated the document since the start of the web request.

Chris Marisic

unread,
Oct 21, 2014, 12:56:30 PM10/21/14
to rav...@googlegroups.com


On Tuesday, October 21, 2014 12:40:24 PM UTC-4, Gareth Thackeray wrote:
We have the following:

web request starts 
  -> loads document A 
  -> updates document B in separate session 
  -> B updates A through a scripted index results patch 
  -> web request saves A because it was updated the last time this happened and decimals got converted to ints
  -> FAIL, because A got updated by the separate session

What I'm doing is totally unorthodox


Actually this seems to be a very standard cloud pattern. Changes to B result in queued changes to A. Honestly it's just a variant of Pub/Sub and nothing at all unorthodox.

Gareth Thackeray

unread,
Oct 21, 2014, 1:00:29 PM10/21/14
to <ravendb@googlegroups.com>
Hmm it's mainly worrying about you 2 telling me I was doing it wrong that stopped me verifying my solution with the community... well we'll see what happens.

--
You received this message because you are subscribed to a topic in the Google Groups "RavenDB - 2nd generation document database" group.
To unsubscribe from this topic, visit https://groups.google.com/d/topic/ravendb/qXD_U-S4aTY/unsubscribe.
To unsubscribe from this group and all its topics, send an email to ravendb+u...@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.

Chris Marisic

unread,
Oct 21, 2014, 1:05:08 PM10/21/14
to rav...@googlegroups.com
Context is king. The statement "B queues updates to A" is without context and is entirely reasonable. Depending what A & B are, and why is it queuing things, are what determines whether it's a good solution.

Kijana Woodard

unread,
Oct 21, 2014, 1:48:22 PM10/21/14
to rav...@googlegroups.com
I'm not happy about creating that atmosphere. :-/

I think you've definitely hit an edge case presented by the confluence of js and c#.
I'm assuming using parseFloat in your SIR script does nothing?

On your "unorthodox solution", you have two raven sessions in a single web request?
I guess if you "queue" the update to B rather than save it in the same web request [diff session], the problem would go away? I guess if you "Save A" before "updating B" the problem goes away as well, correct?

Are there any intentional changes between Updating B and Saving A?
If so, could you evict A from the first session and reload A? Then you should have the latest, but I guess this is a race with SIR.

To be deterministic, you're going to have to flow the updates intentionally.



--
You received this message because you are subscribed to the Google Groups "RavenDB - 2nd generation document database" group.
To unsubscribe from this group and stop receiving emails from it, send an email to ravendb+u...@googlegroups.com.

Gareth Thackeray

unread,
Oct 21, 2014, 4:08:43 PM10/21/14
to <ravendb@googlegroups.com>
I apologise Kijana I think you're a great guy who's very helpful.  I just sometimes feel like there's a bit of an automatic "doing it wrong" assumption round here that can be a bit hard to battle past.  But I don't in any way mean to single you out.

parseFloat wouldn't help.  And in fact the decimals are not even touched by the SIR script.  (A document is loaded, one value is updated then saved again.)  I will post the failing test in a minute - I forgot to include it in the original post.

You are right that saving A before updating B would work just as well as evicting it, but since a large part of the reason for having B is to avoid updating A at this time it's perfectly reasonably to evict it!

Since you seem intrigued I'll give you a flavour of what we're doing:

That's right that we have 2 sessions in this particular web request.  The crux of it is that we are reserving / booking limited-quantity and datetime-based "slots" of a particular resource, as part of booking a holiday.  In order to avoid overbooking, each booked slot is represented by a single document (B) with a conventional id (i.e. so there is a concurrency error in the event of simultaneous booking of the last slots).  Because there can be many resources / slots to book at the same time, we create new sessions so that a) a partial booking can be accomplished b) we can retry within the same web request if the slots we wanted have just been nabbed by someone else and c) because some reading is done and we are working around the requests-per-session limit.

The booking of the slot document then feeds back (via a map-reduce index and SIR) into the overall calendar document (A), which contains aggregate "number remaining" values for that resource over several months.  We need this overall calendar document partly so that we can display a calendar view of availability without needing to query and partly so that we can query for holidays that are still available within a particular date range (by including information from their calendars in the holiday index).

Raven seems to be working well for this so far, but if necessary we will move the link between B and A into some sort of queuing system external to Raven.

G

Gareth Thackeray

unread,
Oct 21, 2014, 4:09:39 PM10/21/14
to <ravendb@googlegroups.com>
Forgot to include the failing test:

        [Test]
        public void FailingTest()
        {
            using (var store = NewDocumentStore())
            {
                using (var session = store.OpenSession())
                {
                    session.Store(new ScriptedIndexResults()
                    {
                        Id = ScriptedIndexResults.IdPrefix + new Foos().IndexName,
                        IndexScript = @"
    var bar = LoadDocument(this.BarId);    
    bar.Wibble = 'hello';
    PutDocument(this.BarId, bar);
",
                    });
                    session.SaveChanges();
                }

                new Foos().Execute(store);

                string id;
                using (var session = store.OpenSession())
                {
                    var foo = new Foo() { Id = "F1", BarId = "B1", Wibble = "hello" };
                    session.Store(foo);

                    var bar = new Bar() { Id = "B1", D = 1.0m };
                    //var bar = new Bar() {Id = "B1", D = 1.5m}; //with this line, the test passes
                    session.Store(bar);

                    session.SaveChanges();
                }

                WaitForIndexing(store);

                using (var session = store.OpenSession())
                {
                    var bar = session.Load<Bar>("B1");
                    var etagBeforeSave = session.Advanced.GetEtagFor(bar);

                    session.SaveChanges();

                    var etagAfterSave = session.Advanced.GetEtagFor(bar);

                    //at this point the session saves a new version of B1 as the decimal was converted to an integer by the ScriptedIndexResult
                    Assert.That(etagBeforeSave, Is.EqualTo(etagAfterSave));
                    Assert.That(session.Advanced.NumberOfRequests, Is.EqualTo(1));
                }
            }
        }

        protected override void ModifyConfiguration(InMemoryRavenConfiguration configuration)
        {
            configuration.Settings["Raven/ActiveBundles"] = "ScriptedIndexResults";
            configuration.Catalog.Catalogs.Add(new TypeCatalog(typeof(ScriptedIndexResultsIndexTrigger)));
        }

        public class Foo
        {
            public string Id { get; set; }
            public string Wibble { get; set; }
            public string BarId { get; set; }
        }

        public class Bar
        {
            public string Id { get; set; }
            public decimal D { get; set; }
            public string Wibble { get; set; }
        }

        public class Foos : AbstractIndexCreationTask<Foo>
        {
            public Foos()
            {
                Map = foos => from foo in foos
                              select new { foo.BarId, foo.Wibble };
            }
        }

--
You received this message because you are subscribed to a topic in the Google Groups "RavenDB - 2nd generation document database" group.
To unsubscribe from this topic, visit https://groups.google.com/d/topic/ravendb/qXD_U-S4aTY/unsubscribe.
To unsubscribe from this group and all its topics, send an email to ravendb+u...@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.

Kijana Woodard

unread,
Oct 21, 2014, 5:24:25 PM10/21/14
to rav...@googlegroups.com
Thanks for that. 

As "a person trying answer questions", it's often difficult to separate "I didn't even bother to google this" from "I have my own baggage from my years of experience and I refuse to see another path" from "I've thought about it from the raven/document point of view and I'm stuck because of xyz". 

As "a person trying to ask questions", it's often difficult to remove noise from the question yet have enough relevant context that people can give an intelligent answer. There are 1000 ways to accomplish most things. 995 are terrible. 5 are generally reasonable and 1 or 2 are probably correct given the full business context. Getting that into a question on a fourm, that people will bother to read, is hard.

Also, we all have bad days. ;-]

Interesting. I need to digest the domain / code logic, but my thoughts go beyond "getting this to work in raven".

My guess is that without WaitForIndexing(store), you're code is racing with SIR so sometimes it would work and sometimes it wouldn't.

I'm unsure what can be done since javascript only has a Number type.

Gareth Thackeray

unread,
Oct 22, 2014, 4:29:26 AM10/22/14
to <ravendb@googlegroups.com>
That's right, in production it will be a race.  But in development it happens every time, which is obviously bad enough!

Interestingly, eval patching doesn't appear to do this - I'm not sure why.  I'm hoping Ayende will give his 2p at some point.

Oren Eini (Ayende Rahien)

unread,
Oct 22, 2014, 5:31:13 AM10/22/14
to ravendb
I just tried to run your test, it passes on 3.0
What build are you running on?

Hibernating Rhinos Ltd  

Oren Eini l CEO Mobile: + 972-52-548-6969

Office: +972-4-622-7811 l Fax: +972-153-4-622-7811

 

ScriptedIndexResultsAndDecimals.cs

Gareth Thackeray

unread,
Oct 22, 2014, 5:32:47 AM10/22/14
to <ravendb@googlegroups.com>
2913

Oren Eini (Ayende Rahien)

unread,
Oct 22, 2014, 5:39:17 AM10/22/14
to ravendb
Okay, I can confirm that this seems to be an issue in 2.5, but it is fixed in 3.0

Can you upgrade?

Gareth Thackeray

unread,
Oct 22, 2014, 5:40:44 AM10/22/14
to <ravendb@googlegroups.com>
That's good news.  We will over the next few months but we have a workaround for now.  Thanks!
Reply all
Reply to author
Forward
0 new messages