Benchmarking - DO NOT MERGE - #4487
Open
prakhargarg105 wants to merge 570 commits into
Open
Conversation
Wire fetchProm into MatrixRunner.Run so each sweep point's prom dump is fetched from S3 (non-fatal on miss), parsed via ParsePromStream, and embedded in SweepPoint.Prom. Propagate Prom from SweepPoint into PointResult in the result assembler so it appears in JSON output. Co-Authored-By: Claude Sonnet 4.6 <[email protected]>
Extend Anomaly with GoroutinesAtStart, HeapInUseMBAtStart, GCPauseDeltaNS fields, add DetectAnomaliesWithProm that populates them from the nearest prom snapshot at anomaly start, and wire it into MatrixRunner.Run. Co-Authored-By: Claude Sonnet 4.6 <[email protected]>
Adds benchmarking/aws/cleanup-lambda as a standalone Go module with: - cleanupAPI interface covering EC2/RDS/S3/IAM/SNS/RGT operations - FakeAWS test double satisfying the interface - stub main() placeholder for Task 6 Co-Authored-By: Claude Sonnet 4.6 <[email protected]>
The accidental go build artifact cleanup-lambda/cleanup-lambda was committed; remove it and extend benchmarking/aws/.gitignore to prevent recurrence. Co-Authored-By: Claude Sonnet 4.6 <[email protected]>
TDD Task 2: add parseARN helper (splits service + resource-id from an ARN across ec2/rds/s3/iam/default shapes) and olderThanTTL predicate, covered by TestParseARN_KnownServices, TestParseARN_Malformed, and TestOlderThanTTL. Pulls testify into the lambda go.mod. Co-Authored-By: Claude Sonnet 4.6 <[email protected]>
TDD: tests added first (red), then processEC2Instance and processRDSInstance implemented to terminate/delete resources that exceed the configured TTL while leaving younger ones untouched (green). Co-Authored-By: Claude Sonnet 4.6 <[email protected]>
Co-Authored-By: Claude Opus 4.7 (1M context) <[email protected]>
Adds the top-level Sweep function that ties discovery, per-resource processors, and SNS notification together; SNS is only published when at least one resource was actually destroyed. Removes now-unnecessary sanity blank-identifier vars. Covered by two new TDD tests. Co-Authored-By: Claude Sonnet 4.6 <[email protected]>
Replaces the stub main.go with the full Lambda handler: builds awsCleanup adapters for all 29 cleanupAPI methods, reads BENCH_ORPHAN_TTL_HOURS and BENCH_ORPHAN_SNS_TOPIC_ARN env vars, and starts the Lambda runtime via aws-lambda-go. Also fixes a pre-existing compile-only issue: extracts the test-only FakeAWS S3 age-check into a narrow s3AgeProvider interface so that sweep.go has no reference to test types and go build ./... succeeds. FakeAWS gains BucketCreatedAt to satisfy the interface; all 9 tests still pass. Co-Authored-By: Claude Sonnet 4.6 <[email protected]>
Add cleanup.tf with the orphan-cleanup Lambda function, IAM role/policy, EventBridge 15-min schedule, and SNS alert topic. Add orphan_ttl_hours variable (default 3h) and orphan_cleanup_sns_topic_arn output. Run terraform fmt on pre-existing files that had drift. Co-Authored-By: Claude Sonnet 4.6 <[email protected]>
Live smoke test deleted a fresh S3 bucket because processS3Bucket relied on a s3AgeProvider type-assertion that only the test FakeAWS satisfied — production awsCleanup never implemented it, so the TTL check was silently skipped and every tagged bucket was unconditionally emptied and deleted. Fix: add ListBuckets to cleanupAPI and awsCleanup, fetch all bucket creation times once at the start of Sweep's S3 phase, pass per-bucket creation time into processS3Bucket so the TTL guard runs in both test and prod. bucketAges is also in scope for the counting pass, replacing the s3AgeProvider hack there too. Drops the s3AgeProvider interface entirely. Adds TestProcessS3Bucket_YoungNotDeleted as a regression test that would have caught the original bug. Co-Authored-By: Claude Opus 4.7 (1M context) <[email protected]>
Bucket-3 item 1 — first proof-point that the framework recipe generalises past postgres. Decisions captured: - Separate seeders/cdc-rows-mysql/ (zero-risk copy/adapt vs dual-driver) - TRUNCATE TABLE orders between sweep points (kills Trap 3) - Connector→engine map in scenario.go; touches 3 sites in main.go - 80K writes/sec mirroring postgres baseline (Trap-1 retry budget if bound) - MySQL 8.0.46, initial_rows: 0, memory checkpoint_cache - backup_retention_period=1 in TF module (CDC binlog retention gotcha) Co-Authored-By: Claude Opus 4.7 (1M context) <[email protected]>
12 tasks: engineSpec map + scripts.go extraction + cache_resources threading + MariaDB client install + rds-mysql TF module + mysql stack + cdc-rows-mysql seeder + scenario YAML + mysql.md skeleton + README update + regression gate + AWS smoke. TDD; commits per task. Co-Authored-By: Claude Opus 4.7 (1M context) <[email protected]>
Add engineSpec + engineSpecs map + engineSpecFor lookup. Validate now rejects scenarios whose connector has no engineSpec entry. Postgres maps as before (DSN-style reset); mysql_cdc lands with discrete host/port/user/pass/db output keys for the mysql CLI. Co-Authored-By: Claude Opus 4.7 (1M context) <[email protected]>
renderSeedScript / combineReset / renderWorkloadScript pulled out of main.go, made engine-aware via engineSpecFor, and unit-tested. Postgres behavior is unchanged (DSN-form reset, POSTGRES_DSN env); mysql_cdc will pick the discrete-flags mysql CLI form. Also fixes the hardcoded 'cdc-rows' binary name in the workload script to use s.Dataset.Seeder. Co-Authored-By: Claude Opus 4.7 (1M context) <[email protected]>
Code review flagged that %q produces Go double-quoted strings — bash still expands $, ` and \ inside "...". Safe today because RDS modules use special=false passwords and reset SQL is hand-authored, but worth documenting before someone wires user input into these renderers. Co-Authored-By: Claude Opus 4.7 (1M context) <[email protected]>
…onfig mysql_cdc requires a checkpoint_cache resource; the scenario YAML now declares one in pipeline.cache_resources and the runner surfaces it at the Connect config root. Postgres scenarios that don't declare one are unchanged. Co-Authored-By: Claude Opus 4.7 (1M context) <[email protected]>
Provides the 'mysql' CLI used by the mysql_cdc reset script (TRUNCATE TABLE orders). AL2023 ships mariadb1011 in the default repo. Co-Authored-By: Claude Opus 4.7 (1M context) <[email protected]>
Mirrors rds-postgres with engine='mysql' and the binlog parameter group needed by mysql_cdc (binlog_format=ROW, binlog_row_image=FULL, binlog_checksum=NONE). Sets backup_retention_period=1 — required for RDS to retain binlog past the moment it's written. Outputs the go-sql-driver DSN plus discrete host/port/user/pass/db pieces for the mysql CLI used in reset scripts. Co-Authored-By: Claude Opus 4.7 (1M context) <[email protected]>
Mirrors stacks/postgres. Stack tag 'mysql' for cost-explorer breakdown. Reads shared VPC/SGs via remote-state and wires them into the rds-mysql module. Exposes the six mysql_* outputs the runner needs for the engineSpec lookup. Co-Authored-By: Claude Opus 4.7 (1M context) <[email protected]>
database/sql + go-sql-driver/mysql variant of cdc-rows. Same CLI shape (seed/workload subcommands), same 8-worker workload pattern. Adapts SQL flavor: BIGINT AUTO_INCREMENT, DATETIME(6), NOW(6), '?' positional placeholders. MYSQL_DSN env var. Co-Authored-By: Claude Opus 4.7 (1M context) <[email protected]>
80K writes/sec → 96 MB/s mirroring the postgres baseline.
initial_rows: 0 + TRUNCATE between points kills Trap 3.
memory checkpoint_cache keeps reset trivial (Connect restart resets it).
tls: { skip_cert_verify: true } because RDS-internal CA isn't on the runner.
Co-Authored-By: Claude Opus 4.7 (1M context) <[email protected]>
The runner appends per-scenario sections to docs/benchmark-results/<stack>.md. This file exists in git so the very first append-run has a non-empty parent file. Local laptop bench results live in mysql-cdc.md (unchanged). Co-Authored-By: Claude Opus 4.7 (1M context) <[email protected]>
…ec extension backup_retention_period=1 is the non-obvious gotcha worth documenting up front. Also updates "Adding a new connector" to mention the engineSpecs map entry (one-line change) — corrects the previous "no runner changes required" claim now that T1+T2 introduced the connector→engine wiring. Co-Authored-By: Claude Opus 4.7 (1M context) <[email protected]>
vCPU 1: 102 MB/s p50 (CPU-bound at 84% of one core, near the mysql_cdc 1-vCPU ceiling) vCPU 2/4: ~101 MB/s p50 (producer-bound at the 80K writes/sec cap — Trap 1; mysql_cdc has tons of headroom: 28-30 goroutines, sub-100 MB heap) vCPU 8: 70 MB/s p50 with wide variance — clean ~100 MB/s for the first 3 minutes then a producer-side step-down to ~50K writes/sec for the remaining 12 minutes. Most likely RDS backup window coinciding with the sweep point. Footnoted; follow-up retry needs backup_window pinned and write_rate bumped to 150K to find real per-vCPU ceiling. Co-Authored-By: Claude Opus 4.7 (1M context) <[email protected]>
ResetScript is rendered once before the sweep, so it must serve every arm in the plan, not just one. Drop and pre-create IcebergResetTables (base + _s0.._s(maxStreams-1)) per engine instead of a single table; the consumer-group reset stays once per engine since both streams of an arm share it.
…guarded The reset runs under set -euo pipefail and iceberg-tablegen exits non-zero on transient Glue/IAM errors (main.go:44,59), while a reset failure aborts the whole sweep (matrix.go:100). The table union turned one unguarded call per engine into N, tripling that exposure. Guard is a bounded retry that still fails loud after 3 attempts, not `|| true`: a silently missing table would make that stream commit nothing and deflate its arm's throughput, converting a loud failure into a wrong A/B conclusion. Found by the Task 6 reviewer as a plan-mandated defect; guard choice confirmed with the user. Co-Authored-By: Claude Opus 5 (1M context) <[email protected]>
…nguarded The union reset (task 6) turned one unguarded tablegen call per engine into N. Under set -euo pipefail, a single throttled Glue/IAM error at reset time would abort the whole sweep. Wrap each pre-create in a bounded 3-attempt retry that still fails loud (exit 1) on genuine failure, rather than `|| true`: a silently missing table would make that stream commit nothing and deflate its arm's throughput instead of erroring, flipping the A/B's conclusion.
…de launch Adds GOMAXPROCS, Streams, RootConfigPath, StreamsDir, and Key to benchScriptArgs so a sweep point can oversubscribe the Go runtime independently of its taskset core pin, and launch `streams -o` instead of `run` when it runs more than one pipeline in-process. Zero values reproduce today's script exactly (parity proven by the pre-existing, unmodified TestRenderBenchScript_EmbedsBucketAndSession).
MatrixRunner.Run now takes []sweepPoint instead of []int, so each measured point carries its own GOMAXPROCS, stream count, and arm-scoped config paths end to end (script render, log/prom fetch, broker-metric fetch). Arm-less plans fall back to the historical single ConfigPath and bare-vCPU artifact keys, so existing scenarios are byte-identical. FakeSSM now records submitted scripts so tests can assert on what the runner actually ran.
…fast The plan claimed `len(out) == 1` preserved the original `n == cpuPoints[0]` semantics. It does not: the original fired for every engine at the first vCPU point, while len(out)==1 fires once per sweep. As the existing scenarios sweep both engines, that silently stops checking Kafka Connect's first point, so a KC misconfig producing zero throughput would burn a full 2-3h sweep instead of aborting at point one. Correct translation is `pt.Key() == plan[0].Key()`. Found by the Task 8 implementer, which flagged it rather than shipping it quietly. Co-Authored-By: Claude Opus 5 (1M context) <[email protected]>
…oint The guard's if len(out) == 1 fired only once per sweep, so it silently stopped checking Kafka Connect's first point in every dual-engine scenario: the original if n == cpuPoints[0] fired once per engine at the first vCPU point. Restore that by keying off pt.Key() == plan[0].Key(), and add a regression test pinning it (a broken second engine's empty first point must abort the sweep, not burn the rest of it).
…root+stream configs
Plan gap: no task wired MetricSidecarArgs.Names to the point's stream count. sinkTopology.MetricSidecar derives its poll list from args.Names.IcebergTables(), so the base m.Names (Streams=0) polls only the unsuffixed table. A 2-stream arm writes _s0/_s1 and would have reported ~0 MB/s with no error — the A/B would read as "2 pipelines are far worse" instead of as a broken measurement. Assigned to Task 8, which owns Run's per-point wiring, with a test asserting the submitted script references both per-stream tables. Found by the Task 9 implementer, which correctly refused to fix it out of scope. Co-Authored-By: Claude Opus 5 (1M context) <[email protected]>
MatrixRunner.Run passed m.Names (Streams == 0) straight into
MetricSidecarArgs, so sinkTopology.MetricSidecar's IcebergTables(engine)
always polled only the unsuffixed base table. A 2-stream arm's two pipelines
commit to ..._s0/..._s1, which the sidecar never looked at — it would poll a
table nothing writes (the reset union still creates it), silently reporting
~0 MB/s with no error raised anywhere. Scope Names via WithStreams(pt.Streams)
so the sidecar polls every table the point actually writes.
fetchBrokerSeriesForEngine's MetricInputs{Names: m.Names} is untouched: the
sink's EngineSeries is ParseIcebergSeries(in.Body), which ignores Names.
…ines Two review findings from Task 9 folded back into the plan: - The upload loop's errors named only the ephemeral local temp path, so a failure could not be attributed to an arm or stream. Both now name the S3 key. - The arms=>--engines=connect guard fired after build, render, stage and seed. Seeding 132 GB is minutes and real spend; the guard needs only s.Matrix.Arms and opts.engines, so it moves to the top of runBench. Co-Authored-By: Claude Opus 5 (1M context) <[email protected]>
…nes mismatch
- os.Open/upload failures in stageArtefacts now include item.key, since
item.path is an ephemeral temp filename that doesn't identify which
arm/stream failed.
- move the matrix.arms requires --engines=connect guard to the top of
runBench, right after scenario load, so an invalid combination fails
before terraform apply/build/render/stage/seed instead of after.
- assert the root config's own placeholder (${REDPANDA_BROKER_ENDPOINTS})
resolves, not just the stream configs'.
… the SHA URL The Task 10 test counted strings.Count(out, "connect") expecting 3, but the template's Git-SHA link (result.md.tmpl:5) contains github.com/redpanda-data/connect/commit/, making the real count 4. Counting "| connect" isolates the table's engine column, so the assertion fails if a row goes missing rather than passing on the URL. Found by the Task 10 implementer, which fixed the assertion instead of bumping the expected number to 4. Co-Authored-By: Claude Opus 5 (1M context) <[email protected]>
…y claim - Spec said to suppress the KC delta column for arms runs; the implementation keeps it, which is correct — arms runs are structurally single-engine, so the column renders blank and the template stays shared. - The plan's "byte-identical rendered scripts" claim was overstated by two intentional, unparsed deviations: the starting-bench echo line and the reset's tablegen retry wrapper. Both raised by the final whole-branch review. Co-Authored-By: Claude Opus 5 (1M context) <[email protected]>
Two intentional, unparsed deviations exist in the rendered script text: the starting-bench echo line and the reset's tablegen retry wrapper. Config content, S3 keys, artifact names and staging paths are unchanged. Raised by the final whole-branch review. Co-Authored-By: Claude Opus 5 (1M context) <[email protected]>
The early-abort guard's "later points would fail the same way" rationale held when points differed only in vCPU, but arms differ in launch mechanism (run vs. streams mode), so one arm succeeding predicts nothing about another. Fire the guard for every point that carries an arm, not just the very first plan point. Also validate up front, before any AWS spend, that MatrixRunner.Run's ConfigPaths covers every plan key, instead of letting a hypothetical miss fall through to configPathsFor's silent legacy-path fallback. SweepPoint now also carries Streams, threaded through to the result JSON in a later commit. Co-Authored-By: Claude Opus 5 (1M context) <[email protected]>
The sidecar accumulated into one summed total_files_size_bytes/total_records
pair per frame, but the plan's own acceptance check for arm B ("did both
streams' tables grow, or did the rebalance starve one of partitions") is
only answerable live, before the Glue database and warehouse bucket are
torn down with the stack. A healthy 8/8 split and a degenerate 16/0 one
produce the identical surviving sum. Emit a per-table
"table_files_size_bytes <table> <bytes>" line inside the polling loop.
Verified inert to ParseIcebergSeries: its prefix diverges from
"total_files_size_bytes " and "total_records " at the first word, so
every switch case misses and the line falls through the default,
confirmed by a parser test asserting byte-identical series with and
without the new lines.
Co-Authored-By: Claude Opus 5 (1M context) <[email protected]>
runnerConfigPaths (the launched path) and stageArtefacts (the download path) independently rebuilt the same "/opt/bench/cfg/<key>" and "stage/cfg/<key>" literals, with nothing asserting they agreed. Extract hostCfgDir/stageCfgPrefix as the single source of truth for each side, and factor stageArtefacts' path-planning into a pure, unit-testable buildStagePlan so a future drift is caught by a test rather than surfacing as a 0 MB/s point from a launch against a path that was never staged. Also clear each point's streams/ directory (rm -rf + mkdir -p, not just mkdir -p) before downloading into it: that directory is session-independent and downloads are per-file `aws s3 cp`, not `sync`, so a --keep re-run with a smaller stream count would otherwise leave a stale stream-N.yaml behind that launches an extra untracked pipeline. Also documents the pre-existing (test-pinned) aliasing constraint in renderPointConfigs' per-stream render loop: topo.Pipeline mutates a shared map in place, so each stream must be marshalled before the next iteration overwrites it, and the loop must never be parallelized. Co-Authored-By: Claude Opus 5 (1M context) <[email protected]>
…eams PointResult gains Streams (populated for every point, including arm-less rows where it is always 1 — matching GOMAXPROCS' existing precedent of never being omitted for arm-less rows) so a result JSON is re-analysable without inferring the stream count from the arm-id naming convention. Also bound Arm.GOMAXPROCS to <= 64 (the largest vCPU count any supported instance type offers; the taskset core pin always follows vCPU, never GOMAXPROCS, so oversubscribing further buys nothing) and Arm.Streams to <= 8 (each stream costs one rendered config, table, and retried iceberg-tablegen pre-create per engine at every reset — a typo like streams: 1000 would multiply real AWS spend at reset time). Co-Authored-By: Claude Opus 5 (1M context) <[email protected]>
WithStreams always resets StreamIndex to 0, so calling WithStream before WithStreams silently drops the index. Every current call site uses the correct order; name the footgun explicitly in both methods' doc comments so a future caller doesn't discover it the hard way. Co-Authored-By: Claude Opus 5 (1M context) <[email protected]>
The Taskfile passed no --engines, so the runner always defaulted to connect,kafka_connect, which hard-fails for matrix.arms scenarios (arms are Connect-only). Add an optional engines= task var that forwards --engines=<value> when set; the default path (no engines= given) renders byte-identical to before, verified with `task --dry` and a shell syntax check on the resulting trailing-continuation line. Co-Authored-By: Claude Opus 5 (1M context) <[email protected]>
Two errors in the handoff, both hit when actually trying to launch: - `task aws:bench` does not resolve from the repo root; the root Taskfile's includes: omit benchmarking/aws/Taskfile.yml, so it must run from that dir. - There is no aws-vault profile named `bench`; it is AWSAdministratorAccess-605419575229. Co-Authored-By: Claude Opus 5 (1M context) <[email protected]>
Run bench-20260804-011911, 110M rows (~132 GB), 3 arms x 15 min, Connect-only.
GOMAXPROCS streams mean p50 p5 p95
a0-1pipe 2 1 68.27 69.63 60.52 75.47
a1-1pipe 4 1 65.87 73.54 18.16 78.08
b-2pipe 4 2 68.03 67.77 64.56 73.99
Answer: splitting into two streams-mode pipelines does NOT raise throughput at
2 vCPU, and neither does GOMAXPROCS oversubscription — all three arms sit within
3.5% on the mean, indistinguishable with one window each.
The real difference is stability. GOMAXPROCS=4 on a single ordered pipeline made
throughput bursty (p5 collapsed to 18 MB/s, mean below median); splitting the
work across two streams restored it (highest p5 of the three, narrowest band) at
the same ceiling. If you oversubscribe GOMAXPROCS, split the work too.
Baseline validation: a0 mean 68.27 vs the published Recipe A 69.1 at 2 vCPU —
within 1.2%, confirming cross-session reproducibility.
Arm B integrity confirmed from the new per-table metric lines: s0 32.29 GB,
s1 32.38 GB (ratio 0.997) — a clean 8/8 partition split, so the summed number
is not masking a degenerate assignment.
Co-Authored-By: Claude Opus 5 (1M context) <[email protected]>
Customer runs 7 topics -> 7 iceberg tables as 7 one-core pipelines (7 cores) and wants fewer. Tests streams-mode (7 streams) vs fan-in (1 pipeline, interpolated table) at 2 and 4 cores. Key design choice: both topologies write the SAME 7 topic-derived tables, so the sidecar sums an identical set and the reset creates one set — apples-to-apples. Fan-in batches x7 because router.go writes per-table groups sequentially. Co-Authored-By: Claude Opus 5 (1M context) <[email protected]>
dataset.topics (default 1, so every existing scenario is byte-identical) plus partitions_per_topic. BenchNames gains Topics/TopicIndex with _t<i> suffixing for source topic, iceberg table and consumer group, mirroring the existing stream suffixing. Seed script emits one seeder invocation per topic; reset pre-creates the union of topic tables; the metric sidecar sums across them and keeps the per-table lines, which with 7 tables are the only way to see a starved topic. Also lifts the arms single-cpu_points restriction — topology x core count is the whole question for the 7-table consolidation test — replaced by a len(cpu_points)*len(arms) <= 8 guard so a careless scenario cannot commit to a day-long run. Substrate only: streams/fan-in rendering lands next. Co-Authored-By: Claude Opus 5 (1M context) <[email protected]>
…solidation
Implements Task B of the iceberg-7table-consolidation design: Arm.FanIn
(one pipeline subscribed to all N topics, table routed by an interpolated
${! @kafka_topic...} expression) and the streams-mode fix so each stream
in a multi-topic scenario reads its own topic/group/table (_t<i>) instead
of the single-topic _s<i> sharing. A test proves the fan-in interpolation
and streams7 derive byte-identical table names, which is what makes the
two topologies comparable.
Co-Authored-By: Claude Opus 5 (1M context) <[email protected]>
…d 4 cores) Customer runs 7 topics -> 7 iceberg tables as 7 one-core pipelines and wants fewer cores. 119M rows (7 x 17M exactly, ~143 GB) across 7 topics, 4 measured points. Both arms write the same 7 topic-derived tables so the sidecar sums an identical set. Resources held constant (streams7 divides the 500 MiB buffer and 16 in-flight by 7). fanin batches x7 because router.go writes per-table groups sequentially — without that we would measure our own misconfiguration. Co-Authored-By: Claude Opus 5 (1M context) <[email protected]>
…fan-in does not Run bench-20260804-051946, 119M rows across 7 topics (~143 GB), 7 topics -> 7 tables, Connect-only, 15-min windows. arm cores mean p50 p5 p95 streams7 2 62.85 62.71 58.46 69.02 fanin 2 52.66 52.64 46.52 57.99 streams7 4 121.98 120.27 112.98 133.49 fanin 4 49.47 53.32 18.14 72.67 streams mode scales near-linearly with cores (1.94x from 2 to 4) and is steady (p5 113 at 4 cores). Fan-in is FLAT — 52.66 to 49.47, i.e. more cores bought nothing — and got burstier (p5 collapsed to 18). At 4 cores streams mode is 2.5x fan-in on the mean. Mechanism is in the code, not a mystery: internal/impl/iceberg/router.go groups a batch per table but writes the groups SEQUENTIALLY, so fan-in's write path cannot use extra cores. max_in_flight only overlaps whole batches. Seven streams get seven genuinely concurrent commit paths instead. Batching x7 removed the small-commit penalty but could not recover the lost parallelism. Customer guidance: 7 streams on 4 cores moves ~122 MB/s across all 7 tables — 43% fewer cores than their current 7x1-core setup. Do NOT use fan-in for this. Co-Authored-By: Claude Opus 5 (1M context) <[email protected]>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
No description provided.