Skip to content

Sample each edge type with its own limit, 10 types per request - #2279

Merged
kmcginnes merged 1 commit into
mainfrom
edge-stack/4-sample-edge-connections-per-type
Sep 26, 2026
Merged

kmcginnes merged 1 commit into
mainfrom
edge-stack/4-sample-edge-connections-per-type

Conversation

@kmcginnes

@kmcginnes kmcginnes commented Sep 25, 2026 •

Copy link
Copy Markdown
Collaborator

Description

This is the fix for #2141 on its own, with the existing batching and no new setting or strategy.

  • The old Gremlin query grouped every edge of a batch before sampling, and Neptune runs that grouping outside its engine, one item per edge. A graph with three edge types over 19.9M edges ran out of memory in about 30 seconds.
  • Each edge type now gets its own limited union() branch, so the 10,000-edge limit applies before anything is read. One limit after hasLabel(A, B, ...) would be shared, and a dominant type would fill it and leave the rest empty.
  • Results are grouped by edge type before counting, because DFE couldn't count one projection across several full branches (two took 54s, five timed out; grouping first handled ten in 9s). The anchor is V().limit(1) because inject() isn't native on Neptune, and mid-traversal V() because E() there needs TinkerPop 3.7.
  • Batches drop from 100 edge types to 10, so a request reads at most 100,000 edges. 100 types in one request took 116s and left a db.t3.medium refusing even single-type samples for two minutes. Other users of DEFAULT_BATCH_REQUEST_SIZE are unchanged.
  • Endpoint labels are folded and every entry is split on ::, so a multi-label vertex keeps every label on Neptune 1.4 (one composite) and 1.3.5 (one entry per label). Before, 1.3.5 silently kept only the first.
  • Dedup uses createEdgeConnectionId, so a label containing the old - separator can't collide.

Validation

  • The query shape matched the true edge connections on Neptune 1.2.1.0 (15/15), 1.3.5.0 (522/522), 1.4.5.1 (2,033/2,033) and 1.4.7.0 (10,018/10,018), and ran on TinkerPop 3.6.2.
  • Four concurrent requests of 10 full edge types each finished in 36s on db.t3.medium instances with and without DFE, and left them answering normally.
  • Tests: template shape and escaping, batches of 10, multi-label composites and one-per-entry labels.
  • pnpm checks and pnpm test clean on this layer.

Related Issues

Check List

  • I confirm that my contribution is made under the terms of the Apache 2.0 license.
  • I have verified pnpm checks passes with no errors.
  • I have verified pnpm test passes with no failures.
  • I have covered new added functionality with unit tests if necessary.
  • I have updated documentation if necessary.

@kmcginnes
kmcginnes added this pull request to stack #2283 September 25, 2026 22:25
kmcginnes added a commit that referenced this pull request Sep 25, 2026
## Description

- `mapWithConcurrency` rejected on the first failure, but its other
lanes kept looping and sending requests nobody was waiting for. The
first rejection now stops the pool from starting new work. Callbacks
already running aren't cancelled.
- This also affects openCypher and SPARQL schema and edge connection
fetches, which used to drain the whole queue after a failure.
- Later in the stack, edge connection discovery relies on this, so
abandoning an attempt actually stops it.

## Validation

- New test: after one callback rejects, no further callbacks start.
- `pnpm checks` and `pnpm test` clean on this layer.

## Related Issues

- Part of #2141 (split out of #2244)
- Layer 1 of 6. Stack, bottom first: #2281 → #2277 → #2278 → #2279 →
#2280 → #2282

### Check List

- [x] I confirm that my contribution is made under the terms of the
Apache 2.0 license.
- [x] I have verified `pnpm checks` passes with no errors.
- [x] I have verified `pnpm test` passes with no failures.
- [x] I have covered new added functionality with unit tests if
necessary.
- [x] I have updated documentation if necessary.
kmcginnes added a commit that referenced this pull request Sep 25, 2026
## Description

- `mapWithConcurrency` rejected on the first failure, but its other
lanes kept looping and sending requests nobody was waiting for. The
first rejection now stops the pool from starting new work. Callbacks
already running aren't cancelled.
- This also affects openCypher and SPARQL schema and edge connection
fetches, which used to drain the whole queue after a failure.
- Later in the stack, edge connection discovery relies on this, so
abandoning an attempt actually stops it.

## Validation

- New test: after one callback rejects, no further callbacks start.
- `pnpm checks` and `pnpm test` clean on this layer.

## Related Issues

- Part of #2141 (split out of #2244)
- Layer 1 of 6. Stack, bottom first: #2281 → #2277 → #2278 → #2279 →
#2280 → #2282

### Check List

- [x] I confirm that my contribution is made under the terms of the
Apache 2.0 license.
- [x] I have verified `pnpm checks` passes with no errors.
- [x] I have verified `pnpm test` passes with no failures.
- [x] I have covered new added functionality with unit tests if
necessary.
- [x] I have updated documentation if necessary.
@kmcginnes
kmcginnes force-pushed the edge-stack/4-sample-edge-connections-per-type branch from 8fd035a to f63dd71 Compare September 25, 2026 22:46
@kmcginnes
kmcginnes force-pushed the edge-stack/4-sample-edge-connections-per-type branch from f63dd71 to e238290 Compare September 25, 2026 23:16
@kmcginnes
kmcginnes force-pushed the edge-stack/4-sample-edge-connections-per-type branch 2 times, most recently from 24e4e84 to 55a3574 Compare September 25, 2026 23:40

@kmcginnes kmcginnes left a comment

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Approved

@kmcginnes
kmcginnes marked this pull request as ready for review September 25, 2026 23:42
@kmcginnes
kmcginnes force-pushed the edge-stack/4-sample-edge-connections-per-type branch from 55a3574 to f569cbb Compare September 26, 2026 17:48
@kmcginnes
kmcginnes force-pushed the edge-stack/4-sample-edge-connections-per-type branch from f569cbb to d01f960 Compare September 26, 2026 21:57
@kmcginnes
kmcginnes removed this pull request from stack #2283 September 26, 2026 23:14
@kmcginnes
kmcginnes force-pushed the edge-stack/4-sample-edge-connections-per-type branch from d01f960 to b3fe4ce Compare September 26, 2026 23:15
@kmcginnes
kmcginnes changed the base branch from edge-stack/3-advanced-connection-options to edge-stack/2-schema-view-without-edge-connections September 26, 2026 23:15
@kmcginnes
kmcginnes added this pull request to stack #2289 September 26, 2026 23:15
kmcginnes added a commit that referenced this pull request Sep 26, 2026
## Description

- Fetch Timeout and Neighbor Expansion Limit move into a collapsible
"Advanced options" section of the connection dialog, so the common
fields aren't buried.
- The section opens by itself when the connection already overrides one
of them, so an existing override is never hidden.
- The trigger is a real button, so it works from the keyboard and
announces whether it's expanded.
- The Fetch Timeout error message and the troubleshooting guide now
point at the connection's advanced options.

## Validation

- Tests that the section starts collapsed, expands on click, and opens
itself for a connection with a Fetch Timeout override.
- `pnpm checks` and `pnpm test` clean on this layer.

## Screenshots

A new connection starts with the advanced options collapsed.

<img width="720" alt="New connection dialog with Advanced options
collapsed"
src="https://github.com/user-attachments/assets/134122d6-9eaf-40ef-ba2d-09ae22516c85"
/>

Expanding them shows Fetch Timeout and Neighbor Expansion Limit.

<img width="720" alt="Advanced options expanded, showing Enable Fetch
Timeout and Override Default Neighbor Expansion Limit"
src="https://github.com/user-attachments/assets/77f7d6d5-3dce-4c6c-b33e-55eecc8b71df"
/>

Editing a connection that already sets a Fetch Timeout opens the section
by itself.

<img width="720" alt="Editing a connection with a 30000 ms fetch
timeout, with Advanced options already open"
src="https://github.com/user-attachments/assets/dd71d9cd-ca86-45c7-8ffd-ff0af44b387d"
/>

## Related Issues

- Part of #2141 (split out of #2244)
- Layer 1 of 5. Stack, bottom first: #2278 → #2277 → #2279 → #2280 →
#2282

### Check List

- [x] I confirm that my contribution is made under the terms of the
Apache 2.0 license.
- [x] I have verified `pnpm checks` passes with no errors.
- [x] I have verified `pnpm test` passes with no failures.
- [x] I have covered new added functionality with unit tests if
necessary.
- [x] I have updated documentation if necessary.
@kmcginnes
kmcginnes removed this pull request from stack #2289 September 26, 2026 23:34
@kmcginnes
kmcginnes changed the base branch from edge-stack/2-schema-view-without-edge-connections to main September 26, 2026 23:34
@kmcginnes
kmcginnes force-pushed the edge-stack/4-sample-edge-connections-per-type branch from b3fe4ce to 5dbbc40 Compare September 26, 2026 23:34
@kmcginnes
kmcginnes added this pull request to stack #2290 September 26, 2026 23:34
@kmcginnes
kmcginnes merged commit 23e3a00 into main Sep 26, 2026
10 checks passed
@kmcginnes
kmcginnes deleted the edge-stack/4-sample-edge-connections-per-type branch September 26, 2026 23:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[3.2.2][Gremlin] Relationship discovery fails with MemoryLimitExceededException for high-cardinality edge labels

1 participant