I had a little bit of time this weekend so I've updated the PR that I'd
submitted https://github.com/apache/tinkerpop/pull/838 with something that
seems to work, at least with the cursory testing I've done so far and jives
better with the sentiments expressed in this thread.
It still uses a similar overall design for the RepeatStep, but it checks to
see if there's a barrier step `.repeat(....barrier())...` as Daniel had
suggested closer to the start of the thread instead of an explicit
`order(BFS)`. So the check wouldn't be, as Pieter was concerned about,
checking that a strategy hasn't been added but that no strategies have
added a `barrier` step to the repeat traversal in the RepeatStep.
On Thu, May 17, 2018 at 3:18 PM Stephen Mallette <spmalle...@gmail.com>
wrote:
> I got a bit busy for a few weeks and didn't' really have much time to focus
> on in on this thread. I figured I'd let it off my mind for a while and see
> if any new thoughts came up on it. Coming back on it now, I still think I
> like the approach I summarized from my own thoughts and others on the
> thread, but let me try to address a few things that were posted since my
> last reply:
>
> From pieter's last reply:
>
> > What bothers me with this is that its in the negative. Providers now
> have too look for a specific strategy NOT being present to alter the
> behavior of the traversal.
>
> I suppose sqlg has gone down a path where the general strategies of
> TinkerPop don't always equate to the best way to do things. Do any other
> graph providers run into such an issue? I don't necessarily ask that
> question of Pieter, but of any graph providers out there who are following
> along. I'm personally not aware of any, but I'd be curious to know if there
> were others.
>
> > Another thought about if we go with the `.withoutStrategies(Lazy...)`.
> Should these semantic marker type strategies not then be interfaces rather
> than concrete implementations.
>
> that might be true. we'd need to look at that more carefully in the
> implementation to see what makes sense. maybe we just need a marker
> interface for such strategies?
>
> From Keith's last reply:
>
> > my main concern is there’s not really any great documentation around
> what LazyBarrierStrategy really does.
>
> true - we will want to write more about this for sure if it's to control
> the traversal algorithm.
>
> On Tue, May 1, 2018 at 9:53 AM, Keith Lohnes <lohn...@gmail.com> wrote:
>
> > I presume that would be some source of technical derailment to this
> > approach
> >
> > Not necessarily, just the “step to define traversal order” idea. The
> > RepeatStep, at least from what I’ve seen, seems simplest to implement as
> a
> > BFS. BFS is easily made depth first with a stack. I think that can still
> be
> > the case with the RepeatStep here, it would just need another means of
> > determining which algorithm to use. That could probably be “Does this
> > repeat traversal end with a barrier step?” If that seems reasonable for
> > RepeatSteps then maybe that could help with some of Pieter’s “check for a
> > negative” concerns.
> >
> > Full BFS: manually add barrier()‘s
> >
> > Mixed mode: Default, let strategies do their thing OR remove strategies
> and
> > manually add your own barrier()
> > Full DFS: execute .withoutStrategies(Lazy...)
> >
> > This sounds reasonable, but my main concern is there’s not really any
> great
> > documentation around what LazyBarrierStrategy really does. Sure, you can
> > read the code, but I think that as the documentation currently sits,
> > inserts
> > barrier()-steps into a traversal where appropriate in order to gain the
> > "bulking optimization." is a bit vague. “Where appropriate” should
> probably
> > be well defined in the documentation so users can have a good
> understanding
> > of when that is without having to go to the code.
> >
> > On Sun, Apr 29, 2018 at 6:17 AM pieter gmail <pieter.mar...@gmail.com>
> > wrote:
> >
> > So to clarify, you write,
> > >
> > > Full DFS: execute `.withoutStrategies(Lazy...)`
> > >
> > > What bothers me with this is that its in the negative. Providers now
> > > have too look for a specific strategy NOT being present to alter the
> > > behavior of the traversal.
> > >
> > > SubgraphStrategy and EventStrategy and all others that I am aware of
> are
> > > positive. They do what they do and then the provider goes about its
> > > business optimizing the steps that are present. In the negative its
> more
> > > like a directive to the provider about the semantics of the traversal's
> > > steps.
> > >
> > > Another thought about if we go with the `.withoutStrategies(Lazy...)`.
> > > Should these semantic marker type strategies not then be interfaces
> > > rather than concrete implementations. Specifically with respect my
> vague
> > > understanding of whats coming with version 4, where the gremlin
> > > specification is completely detached from TinkerPop's reference
> > > implementation and perhaps even the JDK itself.
> > >
> > > Cheers
> > > Pieter
> > >
> > > On 29/04/2018 00:38, Stephen Mallette wrote:
> > > >> Is the objection to order(SearchAlgo) that it overloads order() or
> > an
> > > > objection to specifying, DFS/BFS in the traversal itself?
> > > >
> > > > for me, order() is a step. not a modulator. making it work as
> presented
> > > in
> > > > the pull request conflates those two concepts which i think is
> > confusing
> > > (i
> > > > don't think we do that anywhere else, but feel free to correct me if
> > i'm
> > > > wrong).
> > > >
> > > >> As mentioned earlier I am not a fan of making the user deal with
> > > > strategies. Strategies to my mind is not part of the gremlin as a
> > > language
> > > > specification
> > > >
> > > > There are already precedents for users dealing with strategies.
> > Consider
> > > > the decorations like SubgraphStrategy, EventStrategy, etc. So, it is
> > not
> > > > foreign or hidden. It is a component of traversal configuration.
> > > >
> > > > I would have to think about a new step like traverse(). I'd prefer to
> > > avoid
> > > > new steps when we already have other infrastructure from the language
> > > that
> > > > can support what we're trying to do. I think that's why I like the
> > > approach
> > > > I outlined in my last post - it just sorta fits with what we have.
> > > >
> > > >
> > > >
> > > > On Sat, Apr 28, 2018 at 1:37 PM, pieter gmail <
> pieter.mar...@gmail.com
> > >
> > > > wrote:
> > > >
> > > >> Hi,
> > > >>
> > > >> Is the objection to order(SearchAlgo) that it overloads order() or
> an
> > > >> objection to specifying, DFS/BFS in the traversal itself?
> > > >> If so I do not really see how it is misplaced from a usability/API
> > > >> perspective. Seems pretty natural to me and very graphy at that.
> > > >>
> > > >> As mentioned earlier I am not a fan of making the user deal with
> > > >> strategies. Strategies to my mind is not part of the gremlin as a
> > > language
> > > >> specification. You mention LazyBarrierStrategy yet it is a internal
> > > >> strategy to TinkerPop. Sqlg removes it and passes all tests without
> me
> > > even
> > > >> being aware anymore if TinkerPop would have had a
> LazyBarrierStrategy
> > or
> > > >> not. I see this as a strength of TinkerPop. Strategies can evolve
> over
> > > time
> > > >> without it having a effect on the specification and user's codebase
> > out
> > > >> there.
> > > >>
> > > >> Here comes where my thoughts have gone to,
> > > >>
> > > >> g.V().anythingTraversal(anyTraversal()).traverse(DFS/BFS)
> > > >>
> > > >> This should apply to all traversals, not just RepeatStep. e.g.
> > > >> g.V().out().out().traverse(DFS).
> > > >> If no traverse(DFS/BFS) is specified then the graph provider can do
> > > >> whatever they want, DFS, BFS or a mix of the two.
> > > >> If however traverse(BFS/DFS) is specified then the graph provider
> must
> > > >> follow the directive and the test suite will test for it.
> > > >>
> > > >> traverse(DFS/BFS) is like any other traversal in that it can be
> > > specified
> > > >> mid traversal. Everything before it must follow the directive,
> > > everything
> > > >> after need not to.
> > > >>
> > > >> This will also be backward compatible which is kinda nice.
> > > >>
> > > >> Cheers
> > > >> Pieter
> > > >>
> > > >> On 27/04/2018 21:03, Stephen Mallette wrote:
> > > >>
> > > >>> It seems like we have general agreement on the easy things, that
> is:
> > > >>>
> > > >>> 1. this is a change for 3.4.0/master and
> > > >>> 2. we're all for a DFS option
> > > >>>
> > > >>> but we still have the hard part of having to come to consensus on
> how
> > > it's
> > > >>> used/implemented. The quick summary of this thread in that regard
> > goes
> > > >>> something like this: We currently have this PR that introduces DFS,
> > but
> > > >>> does so as a configuration per repeat() step. From what I gather a
> > good
> > > >>> many of us seem to find that approach undesirable for one or more
> of
> > > the
> > > >>> following reasons:
> > > >>>
> > > >>> 1. The use of order() seems misplaced purely from a usability/API
> > > >>> perspective
> > > >>> 2. The approach seems to be at odds with how everything else works
> > > given
> > > >>> barrier() and strategies
> > > >>> 3. The approach seems to be at odds with our current mixed mode of
> > > DFS/BFS
> > > >>>
> > > >>> I think that we can see those issues resolve themselves with
> > something
> > > >>> Kuppitz mentioned to me: repeat() should be DFS by default where
> > > barrier()
> > > >>> will change that behavior as required. That change would yield the
> > > >>> following approaches:
> > > >>>
> > > >>> Full BFS: manually add `barrier()`'s
> > > >>> Mixed mode: Default, let strategies do their thing OR remove
> > strategies
> > > >>> and
> > > >>> manually add your own barrier()
> > > >>> Full DFS: execute `.withoutStrategies(Lazy...)`
> > > >>>
> > > >>> Futherrmore, we probably should have some form of verification
> > strategy
> > > >>> that ensures all BFS or all DFS so that users can't get tricked
> along
> > > the
> > > >>> way. It's not enough to just remove LazyBarrierStrategy to get DFS
> if
> > > >>> another strategy comes along and throws in a barrier().
> > > >>>
> > > >>> So if all that sounds good from a usability perspective, then we
> get
> > > all
> > > >>> three modes that we want using existing traversal semantics which
> > > removes
> > > >>> the three concerns I've summarized from this thread. We also get
> > > Keith's
> > > >>> desire to have control over which part of a traversal is BFS/DFS if
> > > users
> > > >>> want that capability because they can do a manual Mixed Mode and
> add
> > > their
> > > >>> own barrier() to control the flow. For Pieter (or any graph
> provider)
> > > >>> nothing really changes and there is opportunity to control flow
> with
> > > >>> strategies as usual.
> > > >>>
> > > >>> I haven't really focused much on what's involved in adapting the
> > > current
> > > >>> work in the PR to this approach as I more wanted to find the common
> > > ground
> > > >>> among all the people who commented on the thread. If we agree that
> > > this is
> > > >>> a nice way to go, then we can think more about "how" it could
> happen.
> > > >>>
> > > >>> Keith, I saw you mention earlier that:
> > > >>>
> > > >>> The barrier step that Daniel described doesn’t currently work
> > since
> > > >>> there’s basically booleans in the RepeatStep on whether or not to
> > stash
> > > >>> the
> > > >>> starts to make the RepeatStep depth first.
> > > >>>
> > > >>> I presume that would be some source of technical derailment to this
> > > >>> approach.
> > > >>>
> > > >>>
> > > >>>
> > > >>>
> > > >>> On Tue, Apr 24, 2018 at 3:05 PM, Keith Lohnes <lohn...@gmail.com>
> > > wrote:
> > > >>>
> > > >>> Yeah, that's what I meant. The steps inside are replaced with some
> > > >>>> JanusGraph stuff.
> > > >>>>
> > > >>>> Cheers,
> > > >>>> Keith
> > > >>>>
> > > >>>>
> > > >>>> On Tue, Apr 24, 2018 at 1:52 PM pieter gmail <
> > pieter.mar...@gmail.com
> > > >
> > > >>>> wrote:
> > > >>>>
> > > >>>> Nah, that looks to me like the RepeatStep survived. Just the
> nested
> > > >>>>> VertexStep that got replaced with JanusgraphVertexStep.
> > > >>>>> Good for them, first prize is not replacing anything.
> > > >>>>>
> > > >>>>> Cheers
> > > >>>>> Pieter
> > > >>>>>
> > > >>>>> On 24/04/2018 19:50, Keith Lohnes wrote:
> > > >>>>>
> > > >>>>>> It looks like it,
> > > >>>>>> `g.V().has("foo", "bar").repeat(out()).emit().explain()`
> > > >>>>>>
> > > >>>>>> yields
> > > >>>>>>
> > > >>>>>> `[JanusGraphStep([],[foo.eq(bar)]),
> > > >>>>>> RepeatStep([JanusGraphVertexStep(OUT,vertex),
> > > >>>>>> RepeatEndStep],until(false),emit(true))]`
> > > >>>>>>
> > > >>>>>>
> > > >>>>>>
> > > >>>>>> On Tue, Apr 24, 2018 at 12:12 PM pieter gmail <
> > > pieter.mar...@gmail.com
> > > >>>>>> wrote:
> > > >>>>>>
> > > >>>>>> Hi,
> > > >>>>>>> Sqlg completely replaces TinkerPop's RepeatStep. The idea being
> > > that
> > > >>>>>>> with g.V().repeat(out()).times(x) only x round trips to the db
> is
> > > >>>>>>>
> > > >>>>>> needed
> > > >>>>> regardless of the size of the graph. Each time it will go to the
> db
> > > >>>>>> with
> > > >>>>> the full set of the previous step's incoming starts.
> > > >>>>>>> But yeah TinkerPop's implementation is always the starting
> point
> > so
> > > >>>>>>>
> > > >>>>>> I'll
> > > >>>>> definitely have a look at how you have implemented DFS.
> > > >>>>>>> BTW, does Janus graph use TinkerPop's default RepeatStep as is
> > > with no
> > > >>>>>>> optimization strategies?
> > > >>>>>>>
> > > >>>>>>> Cheers
> > > >>>>>>> Pieter
> > > >>>>>>>
> > > >>>>>>> On 24/04/2018 16:33, Keith Lohnes wrote:
> > > >>>>>>>
> > > >>>>>>>> Pieter,
> > > >>>>>>>>
> > > >>>>>>>> If you take a look at
> > > https://github.com/apache/tinkerpop/pull/838
> > > >>>>>>>>
> > > >>>>>>> DFS
> > > >>>>> is
> > > >>>>>>>> implemented as a modification to BFS. It's taking the starts
> > that
> > > >>>>>>>>
> > > >>>>>>> come
> > > >>>>> in
> > > >>>>>
> > > >>>>>> from a BFS and stashing them to be processed later. I haven't
> > seen a
> > > >>>>>>> big
> > > >>>>>> performance difference on JanusGraph; At least for the queries
> > that
> > > >>>>>>> I've
> > > >>>>>> been running with it. I'm not terribly familiar with Sqlg, but I
> > > >>>>>>> wonder
> > > >>>>> if
> > > >>>>>>>> in the context of how DFS is implemented there, it may be less
> > of
> > > a
> > > >>>>>>>> concern.
> > > >>>>>>>>
> > > >>>>>>>> Cheers,
> > > >>>>>>>> Keith
> > > >>>>>>>>
> > > >>>>>>>> On Thu, Apr 19, 2018 at 12:46 PM pieter gmail <
> > > >>>>>>>>
> > > >>>>>>> pieter.mar...@gmail.com
> > > >>>>> wrote:
> > > >>>>>>>> Hi,
> > > >>>>>>>>> Not really sure either what 'global' means technically with
> > > respect
> > > >>>>>>>>>
> > > >>>>>>>> to
> > > >>>>> TinkerPop's current configuration support.
> > > >>>>>>>>> Perhaps it can be the start of a global configuration
> registry
> > > that
> > > >>>>>>>>>
> > > >>>>>>>> can
> > > >>>>>> be overridden per traversal.
> > > >>>>>>>>> I get that DFS is the preferred default but for Sqlg the
> > > performance
> > > >>>>>>>>> impact is so great that I'd rather, if possible have BFS as
> its
> > > >>>>>>>>>
> > > >>>>>>>> default.
> > > >>>>>> I am not sure about this but I reckon that any graph where the
> > > >>>>>>>> TinkerPop
> > > >>>>>> vm is not running in the same process space as the actual
> > graph/data
> > > >>>>>>>>> that latency is a big issue. BFS alleviates the latency issue
> > > >>>>>>>>> significantly.
> > > >>>>>>>>>
> > > >>>>>>>>> Cheers
> > > >>>>>>>>> Pieter
> > > >>>>>>>>>
> > > >>>>>>>>>
> > > >>>>>>>>>
> > > >>>>>>>>> On 19/04/2018 14:49, Keith Lohnes wrote:
> > > >>>>>>>>>
> > > >>>>>>>>>> whether this will affect more than just
> > > >>>>>>>>>> repeat()?
> > > >>>>>>>>>>
> > > >>>>>>>>>> For the PR that I start and the intent of this thread was to
> > > only
> > > >>>>>>>>>>
> > > >>>>>>>>> affect
> > > >>>>>>>> repeat.
> > > >>>>>>>>>> I prefer that the semantics of the traversal be specified in
> > the
> > > >>>>>>>>>> traversal
> > > >>>>>>>>>> as a first class citizen.
> > > >>>>>>>>>>
> > > >>>>>>>>>> +1
> > > >>>>>>>>>>
> > > >>>>>>>>>> I am fine with any default but am wondering whether it
> > > would
> > > >>>>>>>>>> be
> > > >>>>> worthwhile for the default to be overridden at a global level at
> > > >>>>>>>>> not
> > > >>>>> just
> > > >>>>>>>> per traversal
> > > >>>>>>>>>> It might be nice, but I guess I'm not sure what `global`
> means
> > > in
> > > >>>>>>>>>>
> > > >>>>>>>>> this
> > > >>>>>> context. Configured on the graph object?
> > > >>>>>>>>>> On Tue, Apr 17, 2018 at 9:28 PM Daniel Kuppitz
> > <m...@gremlin.guru
> > > >
> > > >>>>>>>>>>
> > > >>>>>>>>> wrote:
> > > >>>>>>>> TinkerPop makes no guarantees about the order of elements
> unless
> > > >>>>>>>>>> you
> > > >>>>> specify an explicit order. This also goes back to the fact that
> > > >>>>>>>>>> certain
> > > >>>>>>>> strategies (LazyBarrier-, RepeatUnroll- and
> > > >>>>>>>>>> PathRetractionStrategy)
> > > >>>>> add
> > > >>>>>>>> NoOpBarrierSteps to your traversal, which ultimately turns it
> > > >>>>>>>>>> into a
> > > >>>>> DFS/BFS mix. Check the .explain() output of your traversal to see
> > > >>>>>>>>>> which
> > > >>>>>>>> strategy adds which steps.
> > > >>>>>>>>>>> Cheers,
> > > >>>>>>>>>>> Daniel
> > > >>>>>>>>>>>
> > > >>>>>>>>>>>
> > > >>>>>>>>>>> On Tue, Apr 17, 2018 at 4:45 PM, Michael Pollmeier <
> > > >>>>>>>>>>> mich...@michaelpollmeier.com> wrote:
> > > >>>>>>>>>>>
> > > >>>>>>>>>>> Also it seems to me that DFS only really applies to
> repeat()
> > > >>>>>>>>>>>> with
> > > >>>>> an
> > > >>>>>
> > > >>>>>> emit().
> > > >>>>>>>>>>>>> g.V().hasLabel("A").repeat().times(2) gets rewritten as
> > > >>>>>>>>>>>>> g.V().hasLabel("A").out().out(). Are their subtleties
> > that I
> > > am
> > > >>>>>>>>>>>>>
> > > >>>>>>>>>>>> not
> > > >>>>>> aware of or does DFV vs BFS not matter in this case?
> > > >>>>>>>>>>>> When I read this I thought: clearly `.out().out()` is DFS
> > for
> > > >>>>>>>>>>>>
> > > >>>>>>>>>>> OLTP,
> > > >>>>> that's also what the documentation says, e.g. in this nice
> > > >>>>>>>>>>> infographic
> > > >>>>>>>> http://tinkerpop.apache.org/docs/current/images/oltp-vs-
> > olap.png
> > > >>>>>>>>>>>> However, looks like that's not the case. Has my life been
> a
> > > lie?
> > > >>>>>>>>>>>>
> > > >>>>>>>>>>> Setting
> > > >>>>>>>>>> up a simple flat graph to make things more obvious:
> > > >>>>>>>>>>>> v3 <- v1 <- v0 -> v2 -> v4
> > > >>>>>>>>>>>>
> > > >>>>>>>>>>>> ```
> > > >>>>>>>>>>>> graph = TinkerGraph.open()
> > > >>>>>>>>>>>> v0 = graph.addVertex("l0")
> > > >>>>>>>>>>>> v1 = graph.addVertex("l1")
> > > >>>>>>>>>>>> v2 = graph.addVertex("l1")
> > > >>>>>>>>>>>> v3 = graph.addVertex("l2")
> > > >>>>>>>>>>>> v4 = graph.addVertex("l2")
> > > >>>>>>>>>>>> v0.addEdge("e", v2)
> > > >>>>>>>>>>>> v2.addEdge("e", v4)
> > > >>>>>>>>>>>> v0.addEdge("e", v1)
> > > >>>>>>>>>>>> v1.addEdge("e", v3)
> > > >>>>>>>>>>>> g = graph.traversal()
> > > >>>>>>>>>>>> g.V(v0).out().sideEffect{println(it)}.out().sideEffect{
> > > >>>>>>>>>>>>
> > > >>>>>>>>>>> println(it)}
> > > >>>>> ```
> > > >>>>>>>>>>>> Prints:
> > > >>>>>>>>>>>> v[2]
> > > >>>>>>>>>>>> v[1]
> > > >>>>>>>>>>>> v[4]
> > > >>>>>>>>>>>> ==>v[4]
> > > >>>>>>>>>>>> v[3]
> > > >>>>>>>>>>>> ==>v[3]
> > > >>>>>>>>>>>>
> > > >>>>>>>>>>>> If this was OLTP the output would be:
> > > >>>>>>>>>>>> v[2]
> > > >>>>>>>>>>>> v[4]
> > > >>>>>>>>>>>> ==>v[4]
> > > >>>>>>>>>>>> v[1]
> > > >>>>>>>>>>>> v[3]
> > > >>>>>>>>>>>> ==>v[3]
> > > >>>>>>>>>>>>
> > > >>>>>>>>>>>> Cheers
> > > >>>>>>>>>>>> Michael
> > > >>>>>>>>>>>>
> > > >>>>>>>>>>>> On 18/04/18 02:58, pieter gmail wrote:
> > > >>>>>>>>>>>>
> > > >>>>>>>>>>>>> Hi,
> > > >>>>>>>>>>>>>
> > > >>>>>>>>>>>>> I agree with the question about whether this will affect
> > more
> > > >>>>>>>>>>>>>
> > > >>>>>>>>>>>> than
> > > >>>>> just
> > > >>>>>>>>>> repeat()?
> > > >>>>>>>>>>>>> I prefer that the semantics of the traversal be specified
> > in
> > > the
> > > >>>>>>>>>>>>> traversal as a first class citizen. i.e. with
> > > order(SearchAlgo).
> > > >>>>>>>>>>>>> Strategies are to my mind internal to an implementation.
> In
> > > >>>>>>>>>>>>>
> > > >>>>>>>>>>>> Robert's
> > > >>>>>> example LazyBarrierStrategy may be replaced/removed by an
> > > >>>>>>>>>>>> implementation
> > > >>>>>>>>>>>> for whatever internal reason they have.
> > > >>>>>>>>>>>>> Regarding the default, I am fine with any default but am
> > > >>>>>>>>>>>>>
> > > >>>>>>>>>>>> wondering
> > > >>>>> whether it would be worthwhile for the default to be overridden
> > > >>>>>>>>>>>> at a
> > > >>>>>> global level at not just per traversal? That way the impact can
> > > >>>>>>>>>>>> also
> > > >>>>>> be
> > > >>>>>>>>>> alleviated when folks upgrade.
> > > >>>>>>>>>>>>> Also it seems to me that DFS only really applies to
> > repeat()
> > > >>>>>>>>>>>>>
> > > >>>>>>>>>>>> with
> > > >>>>> an
> > > >>>>>
> > > >>>>>> emit().
> > > >>>>>>>>>>>>> g.V().hasLabel("A").repeat().times(2) gets rewritten as
> > > >>>>>>>>>>>>> g.V().hasLabel("A").out().out(). Are their subtleties
> > that I
> > > am
> > > >>>>>>>>>>>>>
> > > >>>>>>>>>>>> not
> > > >>>>>> aware of or does DFV vs BFS not matter in this case?
> > > >>>>>>>>>>>>> Cheers
> > > >>>>>>>>>>>>> Pieter
> > > >>>>>>>>>>>>>
> > > >>>>>>>>>>>>> On 17/04/2018 14:58, Robert Dale wrote:
> > > >>>>>>>>>>>>>
> > > >>>>>>>>>>>>>> +1 on DFS
> > > >>>>>>>>>>>>>> -1 on order(SearchAlgo)
> > > >>>>>>>>>>>>>>
> > > >>>>>>>>>>>>>> It seems like a strategy may be more appropriate. It
> > could
> > > >>>>>>>>>>>>>>
> > > >>>>>>>>>>>>> affect
> > > >>>>>> more
> > > >>>>>>>>>>>> than just repeat(). And how would this interact with
> > > >>>>>>>>>>>>>> LazyBarrierStrategy?
> > > >>>>>>>>>>>>>>
> > > >>>>>>>>>>>>>> Maybe the default should be DFS with
> LazyBarrierStrategy.
> > > Then
> > > >>>>>>>>>>>>>> LazyBarrierStrategy
> > > >>>>>>>>>>>>>> can be removed with 'withoutStrategies()' and then it
> > works
> > > >>>>>>>>>>>>>>
> > > >>>>>>>>>>>>> just
> > > >>>>> like
> > > >>>>>>>> everything else. I'd prefer consistency with the way
> > > >>>>>>>>>>>>> everything
> > > >>>>> else
> > > >>>>>>>> works.
> > > >>>>>>>>>>>>>>
> > > >>>>>>>>>>>>>>
> > > >>>>>>>>>>>>>> Robert Dale
> > > >>>>>>>>>>>>>>
> > > >>>>>>>>>>>>>> On Tue, Apr 17, 2018 at 8:11 AM, Stephen Mallette <
> > > >>>>>>>>>>>>>>
> > > >>>>>>>>>>>>> spmalle...@gmail.com
> > > >>>>>>>>>>>> wrote:
> > > >>>>>>>>>>>>>> Thanks for the summary. Just a quick note - I'd not
> worry
> > > >>>>>>>>>>>>>> about
> > > >>>>> the
> > > >>>>>>>> GLV
> > > >>>>>>>>>>>> tests for now. That part should be easy to sort out. Let's
> > > >>>>>>>>>>>>>> first
> > > >>>>> make
> > > >>>>>>>>>> sure
> > > >>>>>>>>>>>>>>> that we get clear on the other items first before
> digging
> > > too
> > > >>>>>>>>>>>>>>>
> > > >>>>>>>>>>>>>> deeply
> > > >>>>>>>> there.
> > > >>>>>>>>>>>>>>> On an administrative front, I think that this change
> > should
> > > >>>>>>>>>>>>>>>
> > > >>>>>>>>>>>>>> just
> > > >>>>> go
> > > >>>>>>>> to
> > > >>>>>>>>>>>> 3.4.0/master (so it's currently targeted to the right
> branch
> > > >>>>>>>>>>>>>>> actually) as
> > > >>>>>>>>>>>>>>> it sounds like we want DFS to be the default and that
> > could
> > > >>>>>>>>>>>>>>>
> > > >>>>>>>>>>>>>> be a
> > > >>>>> breaking
> > > >>>>>>>>>>>>>>> change as the semantics of the traversal shift a bit.
> So
> > > >>>>>>>>>>>>>>>
> > > >>>>>>>>>>>>>> that's
> > > >>>>> one
> > > >>>>>>>> issue
> > > >>>>>>>>>>>>>>> to make sure we're all happy with.
> > > >>>>>>>>>>>>>>>
> > > >>>>>>>>>>>>>>> A next issue would be the API from the user
> perspective -
> > > >>>>>>>>>>>>>>>
> > > >>>>>>>>>>>>>> thanks
> > > >>>>> for
> > > >>>>>>>> the
> > > >>>>>>>>>>>>> link to that JIRA btw - I see I was involved in that
> > > >>>>>>>>>>>>>> conversation
> > > >>>>>> a
> > > >>>>>>>> little
> > > >>>>>>>>>>>>>>> bit but then dropped off. You'd commented that you
> > > preferred a
> > > >>>>>>>>>>>>>>>
> > > >>>>>>>>>>>>>> per
> > > >>>>>> loop
> > > >>>>>>>>>>>> configuration for search approach which is what you stuck
> > with
> > > >>>>>>>>>>>>>> for
> > > >>>>>> the
> > > >>>>>>>>>>>> implementation. I guess I can see that there could be some
> > > >>>>>>>>>>>>>> value
> > > >>>>> in
> > > >>>>>>>> having
> > > >>>>>>>>>>>>>>> that ability. That said, I'm not sure that I like the
> > > overload
> > > >>>>>>>>>>>>>>>
> > > >>>>>>>>>>>>>> of
> > > >>>>>> order()
> > > >>>>>>>>>>>>>>> as the method for doing that:
> > > >>>>>>>>>>>>>>>
> > > >>>>>>>>>>>>>>> g.V().has("name", "marko").
> > > >>>>>>>>>>>>>>> repeat(__.outE().order().by("weight",
> > > decr).inV()).
> > > >>>>>>>>>>>>>>> emit().
> > > >>>>>>>>>>>>>>> order(SearchAlgo.DFS)
> > > >>>>>>>>>>>>>>>
> > > >>>>>>>>>>>>>>> Still thinking about what else we might do there, but
> > > perhaps
> > > >>>>>>>>>>>>>>>
> > > >>>>>>>>>>>>>> that
> > > >>>>>> can wait
> > > >>>>>>>>>>>>>>> a bit as I still need to study your changes in detail.
> > > >>>>>>>>>>>>>>>
> > > >>>>>>>>>>>>>> Regarding:
> > > >>>>>> The barrier step that Daniel described doesn’t currently
> > > >>>>>>>>>>>>>>> work
> > > >>>>>> since
> > > >>>>>>>>>>>> there’s basically booleans in the RepeatStep on whether or
> > not
> > > >>>>>>>>>>>>>> to
> > > >>>>>> stash the
> > > >>>>>>>>>>>>>>> starts to make the RepeatStep depth first.
> > > >>>>>>>>>>>>>>>
> > > >>>>>>>>>>>>>>> I assume you are referring to these booleans:
> > > >>>>>>>>>>>>>>>
> > > >>>>>>>>>>>>>>> https://github.com/apache/tinkerpop/blob/
> > > >>>>>>>>>>>>>>>
> > > >>>>>>>>>>>>>> fe8ee98f6967c7b0f0ee7f2cf2d105
> > > >>>>>>>>>>>>> 31b68fab8b/gremlin-core/src/main/java/org/apache/
> > > >>>>>>>>>>>>>>> tinkerpop/gremlin/process/traversal/step/branch/
> > > >>>>>>>>>>>>>>>
> > > >>>>>>>>>>>>>> RepeatStep.java#L46-L47
> > > >>>>>>>>>>>>> ?
> > > >>>>>>>>>>>>>>>
> > > >>>>>>>>>>>>>>>
> > > >>>>>>>>>>>>>>> On Tue, Apr 17, 2018 at 7:37 AM, Keith Lohnes <
> > > >>>>>>>>>>>>>>>
> > > >>>>>>>>>>>>>> lohn...@gmail.com>
> > > >>>>>> wrote:
> > > >>>>>>>>>>>>> Stephen, That’s a fair summary. I had an immediate need
> for
> > > >>>>>>>>>>>>>>> it,
> > > >>>>> so
> > > >>>>>>>> I
> > > >>>>>>>>>> developed something based on Michel Pollmeier’s work and a
> > > >>>>>>>>>>>>>>> modification
> > > >>>>>>>>>>>>> to
> > > >>>>>>>>>>>>>>>> the syntax Pieter Martin suggested in the Jira
> > > >>>>>>>>>>>>>>>> <https://issues.apache.org/jira/browse/TINKERPOP-1822
> ?
> > > >>>>>>>>>>>>>>>> focusedCommentId=16233681&page=com.atlassian.jira.
> > > >>>>>>>>>>>>>>>>
> > > >>>>>>>>>>>>>>>>
> > > plugin.system.issuetabpanels%3Acomment-tabpanel#comment-1623
> > > >>>>> 3681>
> > > >>>>>
> > > >>>>>> with DFS being explicit.
> > > >>>>>>>>>>>>>>>> The semantics I used were
> g.V().repeat(out()).order(DFS)
> > ,
> > > >>>>>>>>>>>>>>>>
> > > >>>>>>>>>>>>>>> putting
> > > >>>>>> the
> > > >>>>>>>>>>>> order
> > > >>>>>>>>>>>>>>>> clause outside of the repeat. It made it simpler for
> me
> > to
> > > >>>>>>>>>>>>>>>>
> > > >>>>>>>>>>>>>>> develop
> > > >>>>>>>> and
> > > >>>>>>>>>>>> seemed nice to make that DFS vs BFS explicit. The barrier
> > > >>>>>>>>>>>>>>> step
> > > >>>>> that
> > > >>>>>>>> Daniel
> > > >>>>>>>>>>>>>>>> described doesn’t currently work since there’s
> basically
> > > >>>>>>>>>>>>>>>>
> > > >>>>>>>>>>>>>>> booleans
> > > >>>>>> in
> > > >>>>>>>>>> the
> > > >>>>>>>>>>>>>>>> RepeatStep on whether or not to stash the starts to
> make
> > > the
> > > >>>>>>>>>>>>>>>>
> > > >>>>>>>>>>>>>>> RepeatStep
> > > >>>>>>>>>>>>> depth first.
> > > >>>>>>>>>>>>>>>> I added a test for the DFS, though I’ve had some
> issues
> > > >>>>>>>>>>>>>>>>
> > > >>>>>>>>>>>>>>> getting
> > > >>>>> things to
> > > >>>>>>>>>>>>>>>> run re: .net tests and some other tests timing out. I
> > have
> > > >>>>>>>>>>>>>>>>
> > > >>>>>>>>>>>>>>> some
> > > >>>>> more
> > > >>>>>>>>>> tests
> > > >>>>>>>>>>>>>>>> I'd like to write based off issues that I ran into
> > testing
> > > >>>>>>>>>>>>>>>>
> > > >>>>>>>>>>>>>>> this
> > > >>>>> in
> > > >>>>>>>> the
> > > >>>>>>>>>>>> console (k-ary trees, until/emit before the repeat vs
> > after),
> > > >>>>>>>>>>>>>>> but I
> > > >>>>>>>> really
> > > >>>>>>>>>>>>>>>> wanted to get this out there for people to take a look
> > at
> > > and
> > > >>>>>>>>>>>>>>>>
> > > >>>>>>>>>>>>>>> see
> > > >>>>>> if
> > > >>>>>>>>>> it
> > > >>>>>>>>>>>>> could work out.
> > > >>>>>>>>>>>>>>>>
> > > >>>>>>>>>>>>>>>>
> > > >>>>>>>>>>>>>>>> On Mon, Apr 16, 2018 at 7:29 PM Stephen Mallette <
> > > >>>>>>>>>>>>>>>>
> > > >>>>>>>>>>>>>>> spmalle...@gmail.com>
> > > >>>>>>>>>>>>> wrote:
> > > >>>>>>>>>>>>>>>> Keith, I have to admit that I didn't really
> > follow
> > > this
> > > >>>>>>>>>>>>>>>> general
> > > >>>>>>>> body
> > > >>>>>>>>>>>>> of
> > > >>>>>>>>>>>>>>>> work closely, which include this pull request, the
> > earlier
> > > >>>>>>>>>>>>>>>> one
> > > >>>>> from
> > > >>>>>>>>>> Michael
> > > >>>>>>>>>>>>>>>>> Pollmeier, the JIRA and I think some discussion
> > > somewhere on
> > > >>>>>>>>>>>>>>>>>
> > > >>>>>>>>>>>>>>>> one
> > > >>>>>> of
> > > >>>>>>>>>> the
> > > >>>>>>>>>>>>>>>>> mailing lists. As best I have gathered from what I
> did
> > > >>>>>>>>>>>>>>>>>
> > > >>>>>>>>>>>>>>>> follow, I
> > > >>>>>> feel
> > > >>>>>>>>>>>> like
> > > >>>>>>>>>>>>>>>>> the focus of this work so far was mostly related to
> > "can
> > > >>>>>>>>>>>>>>>>>
> > > >>>>>>>>>>>>>>>> someone
> > > >>>>>> get it
> > > >>>>>>>>>>>>>>>> to
> > > >>>>>>>>>>>>>>>>
> > > >>>>>>>>>>>>>>>>> actually work", but not a lot about other aspects
> like
> > > >>>>>>>>>>>>>>>>>
> > > >>>>>>>>>>>>>>>> testing,
> > > >>>>>> api,
> > > >>>>>>>>>>>> release branch to apply it to, etc. Is that a fair
> depiction
> > > >>>>>>>>>>>>>>>> of
> > > >>>>>> how
> > > >>>>>>>>>> this
> > > >>>>>>>>>>>>>>>> work has developed so far? if so, let's use this
> thread
> > to
> > > >>>>>>>>>>>>>>>> make
> > > >>>>>> sure
> > > >>>>>>>>>>>> we're
> > > >>>>>>>>>>>>>>>>> all on the same page as to how this change will get
> in
> > on
> > > >>>>>>>>>>>>>>>>>
> > > >>>>>>>>>>>>>>>> all
> > > >>>>> those
> > > >>>>>>>>>> sorts
> > > >>>>>>>>>>>>>>>> of issues.
> > > >>>>>>>>>>>>>>>>> btw, thanks to you and michael for sticking at this
> to
> > > come
> > > >>>>>>>>>>>>>>>>>
> > > >>>>>>>>>>>>>>>> to
> > > >>>>> something
> > > >>>>>>>>>>>>>>>> that seems to work. looking forward to the continued
> > > >>>>>>>>>>>>>>>> discussion
> > > >>>>>> on
> > > >>>>>>>> this
> > > >>>>>>>>>>>>>>>>> thread.
> > > >>>>>>>>>>>>>>>>>
> > > >>>>>>>>>>>>>>>>>
> > > >>>>>>>>>>>>>>>>> On Mon, Apr 16, 2018 at 6:54 PM, Michael Pollmeier <
> > > >>>>>>>>>>>>>>>>> mich...@michaelpollmeier.com> wrote:
> > > >>>>>>>>>>>>>>>>>
> > > >>>>>>>>>>>>>>>>> Unsurprisingly I'm also +1 for defaulting to DFS in
> > OLTP.
> > > >>>>>>>>>>>>>>>>> My
> > > >>>>> feeling
> > > >>>>>>>>>>>> is
> > > >>>>>>>>>>>>>>>> that most users currently expect it to be DFS since
> > that's
> > > >>>>>>>>>>>>>>>>> what
> > > >>>>>> the
> > > >>>>>>>>>>>> docs
> > > >>>>>>>>>>>>>>>
> > >
> > >
> >
>
