Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

json: properly report capacity of encoded jsons #25679

Merged
merged 1 commit into from
May 19, 2018

Conversation

jordanlewis
Copy link
Member

Previously, jsonEncoded structs reported their memory size as including
the underlying capacity of their encoded byte slices. This was
incorrect, as usually that underlying slice is an entire kv batch. This
problem led to significant overcounting of the memory used by queries
that read JSON data from disk, which resulted in spurious
BudgetExceededErrors.

Release note (bug fix): prevent spurious BudgetExceededErrors for some
queries that read a lot of JSON data from disk.

@cockroach-teamcity
Copy link
Member

This change is Reviewable

@jordanlewis jordanlewis requested review from knz and removed request for justinj May 18, 2018 21:13
Previously, jsonEncoded structs reported their memory size as including
the underlying capacity of their encoded byte slices. This was
incorrect, as usually that underlying slice is an entire kv batch. This
problem led to significant overcounting of the memory used by queries
that read JSON data from disk, which resulted in spurious
BudgetExceededErrors.

Release note (bug fix): prevent spurious BudgetExceededErrors for some
queries that read a lot of JSON data from disk.

Co-authored-by: Alfonso Subiotto Marqués <alfonso@cockroachlabs.com>
@knz
Copy link
Contributor

knz commented May 19, 2018

@jordanlewis any relationship to #25607, which doesn't use json?

Copy link
Contributor

@knz knz left a comment

Choose a reason for hiding this comment

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

Thanks!

@asubiotto
Copy link
Contributor

@knz unfortunately not, those types seem to have correct Size functions.

Copy link
Contributor

@asubiotto asubiotto left a comment

Choose a reason for hiding this comment

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

LGTM although the tcl test failure is weird, any idea what's up with that?

@jordanlewis
Copy link
Member Author

Almost certainly a flake. Retriggered.

@jordanlewis
Copy link
Member Author

TFTR!

bors r+

craig bot pushed a commit that referenced this pull request May 19, 2018
25679: json: properly report capacity of encoded jsons r=jordanlewis a=jordanlewis

Previously, jsonEncoded structs reported their memory size as including
the underlying capacity of their encoded byte slices. This was
incorrect, as usually that underlying slice is an entire kv batch. This
problem led to significant overcounting of the memory used by queries
that read JSON data from disk, which resulted in spurious
BudgetExceededErrors.

Release note (bug fix): prevent spurious BudgetExceededErrors for some
queries that read a lot of JSON data from disk.

Co-authored-by: Jordan Lewis <jordanthelewis@gmail.com>
@craig
Copy link
Contributor

craig bot commented May 19, 2018

Build succeeded

@craig craig bot merged commit 23280f4 into cockroachdb:master May 19, 2018
craig bot pushed a commit that referenced this pull request May 19, 2018
25719: backport-2.0: json: properly report capacity of encoded jsons r=jordanlewis a=jordanlewis

Backport 1/1 commits from #25679.

/cc @cockroachdb/release

Previously, jsonEncoded structs reported their memory size as including
the underlying capacity of their encoded byte slices. This was
incorrect, as usually that underlying slice is an entire kv batch. This
problem led to significant overcounting of the memory used by queries
that read JSON data from disk, which resulted in spurious
BudgetExceededErrors.

Release note (bug fix): prevent spurious BudgetExceededErrors for some
queries that read a lot of JSON data from disk.

Co-authored-by: Alfonso Subiotto Marqués <alfonso@cockroachlabs.com>

Co-authored-by: Jordan Lewis <jordanthelewis@gmail.com>
@jordanlewis jordanlewis deleted the enc-json-size branch May 21, 2018 03:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants