You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@kudu.apache.org by "Zoltan Chovan (Code Review)" <ge...@cloudera.org> on 2026/04/17 14:32:20 UTC
[kudu-CR] ingest: add C++ TSV loader and schema mapping
Zoltan Chovan has uploaded this change for review. ( http://gerrit.cloudera.org:8080/24201
Change subject: ingest: add C++ TSV loader and schema mapping
......................................................................
ingest: add C++ TSV loader and schema mapping
Add a standalone C++ ingest client, build configuration, and usage docs for loading the ClickHouse hits TSV dataset into Kudu.
This creates a high-throughput native baseline that can be benchmarked and tuned as the foundation for the ingest pipeline work.
Change-Id: I2d3fb6b031b26c4215de58de2162e060132846b4
Made-with: Cursor
---
A examples/ingest/cpp/.gitignore
A examples/ingest/cpp/CMakeLists.txt
A examples/ingest/cpp/README.md
A examples/ingest/cpp/kudu_insert.cc
4 files changed, 1,092 insertions(+), 0 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/01/24201/1
--
To view, visit http://gerrit.cloudera.org:8080/24201
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newchange
Gerrit-Change-Id: I2d3fb6b031b26c4215de58de2162e060132846b4
Gerrit-Change-Number: 24201
Gerrit-PatchSet: 1
Gerrit-Owner: Zoltan Chovan <zc...@cloudera.com>
[kudu-CR] [ingest] add C++ TSV loader and schema mapping
Posted by "Kudu Jenkins (Code Review)" <ge...@cloudera.org>.
Kudu Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/24201 )
Change subject: [ingest] add C++ TSV loader and schema mapping
......................................................................
Patch Set 4: Verified-1
Build Failed
http://jenkins.kudu.apache.org/job/pre_commit/2489/ : FAILURE
--
To view, visit http://gerrit.cloudera.org:8080/24201
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I2d3fb6b031b26c4215de58de2162e060132846b4
Gerrit-Change-Number: 24201
Gerrit-PatchSet: 4
Gerrit-Owner: Zoltan Chovan <zc...@cloudera.com>
Gerrit-Reviewer: Attila Bukor <ab...@apache.org>
Gerrit-Reviewer: Gabriella Lotz <lo...@gmail.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Marton Greber <gr...@gmail.com>
Gerrit-Reviewer: Zoltan Chovan <zc...@cloudera.com>
Gerrit-Reviewer: Zoltan Martonka <zm...@cloudera.com>
Gerrit-Comment-Date: Mon, 20 Apr 2026 10:07:25 +0000
Gerrit-HasComments: No
[kudu-CR] [ingest] add C++ TSV loader and schema mapping
Posted by "Alexey Serbin (Code Review)" <ge...@cloudera.org>.
Alexey Serbin has posted comments on this change. ( http://gerrit.cloudera.org:8080/24201 )
Change subject: [ingest] add C++ TSV loader and schema mapping
......................................................................
Patch Set 8: Code-Review+1
(11 comments)
http://gerrit.cloudera.org:8080/#/c/24201/6/examples/ingest/cpp/kudu_insert.cc
File examples/ingest/cpp/kudu_insert.cc:
http://gerrit.cloudera.org:8080/#/c/24201/6/examples/ingest/cpp/kudu_insert.cc@265
PS6, Line 265: "missing column in table schema: " + string(ingest_schema::kColumns[tsv_idx].name));
: }
: }
:
> right, I changed the primary-key lookup as suggested, the logic is in IsPri
Thank you! This is more consistent now.
http://gerrit.cloudera.org:8080/#/c/24201/8/examples/ingest/cpp/kudu_insert.cc
File examples/ingest/cpp/kudu_insert.cc:
http://gerrit.cloudera.org:8080/#/c/24201/8/examples/ingest/cpp/kudu_insert.cc@168
PS8, Line 168: const auto is_ascii_whitespace = [](const char ch) {
: return ch == ' ' || ch == '\t' || ch == '\n' || ch == '\r';
: };
nit: could std::isspace() serve you better here or only these four whitespace-like characters should be ever trimmed?
std::isspace() also knows about a few more whitespace-like characters in addition to these, but it requires 'unsigned char' cast (which costs nothing):
std::isspace(static_cast<unsigned char>(ch));
https://en.cppreference.com/cpp/string/byte/isspace
http://gerrit.cloudera.org:8080/#/c/24201/8/examples/ingest/cpp/kudu_insert.cc@184
PS8, Line 184: const char* type_name
This doesn't smell good: this is used only in error messages, and it's directly derived from IntType (i.e. it'd possible to use template programming techniques to provide this info instead of passing a parameter).
However, a better alternative might be outputting this error at the call sites of ParseIntegral() and using KUDU_RETURN_NOT_OK_PREPEND to combine an error message with the field name: it seems the name of the column/field is available in that context. An error message with column/field name would be much more actionable than just name of a type when there is 100+ fields per row.
http://gerrit.cloudera.org:8080/#/c/24201/8/examples/ingest/cpp/kudu_insert.cc@202
PS8, Line 202: <=
Shouldn't this be '<', i.e. strictly less? Otherwise, it might add an empty component if the string has less than kExpectedFieldCount fields, it seems.
http://gerrit.cloudera.org:8080/#/c/24201/8/examples/ingest/cpp/kudu_insert.cc@203
PS8, Line 203: const size_t tab = line.find('\t', start);
: const size_t token_len = (tab == string::npos) ? (line.size() - start) : (tab - start);
: if (count >= kExpectedFieldCount) {
: return kExpectedFieldCount + 1;
: }
: (*fields)[count] = string_view(line.data() + start, token_len);
: ++count;
: if (tab == string::npos) {
: break;
: }
: start = tab + 1;
I didn't notice in prior review rounds, but does this code work as expected if the fields are separated by two or more tabs instead of one?
Overall, it looks a bit brittle w.r.t. the spacing between fields. Unless two tabs in a row should result in a NULL value in between them semantically, consider switching to a logic that tracks non-tab characters. For example, use a pair of 'line.find_first_not_of('\n', pos)' followed by 'line.find('\n', pos)' (the latter consumes the 'pos' updated by the former) to separate a token at each iteration of the cycle.
http://gerrit.cloudera.org:8080/#/c/24201/8/examples/ingest/cpp/kudu_insert.cc@248
PS8, Line 248: for (size_t tsv_idx = 0; tsv_idx < kExpectedFieldCount; ++tsv_idx) {
: (*column_index_map)[tsv_idx] = -1;
: }
nit: consider using std::fill() for this
http://gerrit.cloudera.org:8080/#/c/24201/8/examples/ingest/cpp/kudu_insert.cc@263
PS8, Line 263: (*column_index_map)
nit: consider introducing a ref and using it within this function instead of de-referencing the pointer every time
auto& cim = *column_index_map;
...
cim[idx] = x;
http://gerrit.cloudera.org:8080/#/c/24201/8/examples/ingest/cpp/kudu_insert.cc@282
PS8, Line 282: column.field_type
nit: consider replacing this with ingest_schema::kColumns[idx].field_type and removing line 280
http://gerrit.cloudera.org:8080/#/c/24201/8/examples/ingest/cpp/kudu_insert.cc@407
PS8, Line 407: array<string_view, kExpectedFieldCount>
readability nit: if it makes sense, introduce a few plain typedefs for each of array<Type, kExpectedFieldCount> that reflects the way how the container is used in this code (alternatively, introduce a typedef template to encapsulate kExpectedFieldCount)
http://gerrit.cloudera.org:8080/#/c/24201/8/examples/ingest/cpp/kudu_insert.cc@487
PS8, Line 487: cout
Since reporting a warning, should this be 'cerr' instead?
http://gerrit.cloudera.org:8080/#/c/24201/8/examples/ingest/cpp/kudu_insert.cc@558
PS8, Line 558: cout
Since reporting an error, should this be 'cerr' instead?
--
To view, visit http://gerrit.cloudera.org:8080/24201
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I2d3fb6b031b26c4215de58de2162e060132846b4
Gerrit-Change-Number: 24201
Gerrit-PatchSet: 8
Gerrit-Owner: Zoltan Chovan <zc...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <al...@apache.org>
Gerrit-Reviewer: Anonymous Coward (763)
Gerrit-Reviewer: Attila Bukor <ab...@apache.org>
Gerrit-Reviewer: Gabriella Lotz <lo...@gmail.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Marton Greber <gr...@gmail.com>
Gerrit-Reviewer: Zoltan Chovan <zc...@cloudera.com>
Gerrit-Reviewer: Zoltan Martonka <zm...@cloudera.com>
Gerrit-Comment-Date: Wed, 06 May 2026 23:20:59 +0000
Gerrit-HasComments: Yes
[kudu-CR] [ingest] add C++ TSV loader and schema mapping
Posted by "Zoltan Chovan (Code Review)" <ge...@cloudera.org>.
Zoltan Chovan has posted comments on this change. ( http://gerrit.cloudera.org:8080/24201 )
Change subject: [ingest] add C++ TSV loader and schema mapping
......................................................................
Patch Set 4:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/24201/3/examples/ingest/cpp/kudu_insert.cc
File examples/ingest/cpp/kudu_insert.cc:
http://gerrit.cloudera.org:8080/#/c/24201/3/examples/ingest/cpp/kudu_insert.cc@267
PS3, Line 267:
> please remove spaces from empty lines
Done
--
To view, visit http://gerrit.cloudera.org:8080/24201
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I2d3fb6b031b26c4215de58de2162e060132846b4
Gerrit-Change-Number: 24201
Gerrit-PatchSet: 4
Gerrit-Owner: Zoltan Chovan <zc...@cloudera.com>
Gerrit-Reviewer: Attila Bukor <ab...@apache.org>
Gerrit-Reviewer: Gabriella Lotz <lo...@gmail.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Marton Greber <gr...@gmail.com>
Gerrit-Reviewer: Zoltan Chovan <zc...@cloudera.com>
Gerrit-Reviewer: Zoltan Martonka <zm...@cloudera.com>
Gerrit-Comment-Date: Mon, 20 Apr 2026 09:29:22 +0000
Gerrit-HasComments: Yes
[kudu-CR] [ingest] add C++ TSV loader and schema mapping
Posted by "Zoltan Martonka (Code Review)" <ge...@cloudera.org>.
Zoltan Martonka has posted comments on this change. ( http://gerrit.cloudera.org:8080/24201 )
Change subject: [ingest] add C++ TSV loader and schema mapping
......................................................................
Patch Set 4:
(2 comments)
http://gerrit.cloudera.org:8080/#/c/24201/4/examples/ingest/cpp/kudu_insert.cc
File examples/ingest/cpp/kudu_insert.cc:
http://gerrit.cloudera.org:8080/#/c/24201/4/examples/ingest/cpp/kudu_insert.cc@454
PS4, Line 454: KUDU_RETURN_NOT_OK(row->SetString("FromTag", fields[100]));
You should define a kudu_throw_not_ok macro, because you seem to try to catch exceptions, but you return instead. (Or remove the try catch block and just check
http://gerrit.cloudera.org:8080/#/c/24201/4/examples/ingest/cpp/kudu_insert.cc@565
PS4, Line 565: IngestResult ingest_result;
: s = InsertDataFromFile(client, config.file_path, &ingest_result);
: if (!s.ok()) {
: std::cerr << "Error inserting data: " << s.ToString() << std::endl;
: return 1;
: }
Is the application returning 0 ok if some rows had parsing errors and was not sent?
--
To view, visit http://gerrit.cloudera.org:8080/24201
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I2d3fb6b031b26c4215de58de2162e060132846b4
Gerrit-Change-Number: 24201
Gerrit-PatchSet: 4
Gerrit-Owner: Zoltan Chovan <zc...@cloudera.com>
Gerrit-Reviewer: Attila Bukor <ab...@apache.org>
Gerrit-Reviewer: Gabriella Lotz <lo...@gmail.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Marton Greber <gr...@gmail.com>
Gerrit-Reviewer: Zoltan Chovan <zc...@cloudera.com>
Gerrit-Reviewer: Zoltan Martonka <zm...@cloudera.com>
Gerrit-Comment-Date: Mon, 20 Apr 2026 10:59:50 +0000
Gerrit-HasComments: Yes
[kudu-CR] [ingest] add C++ TSV loader and schema mapping
Posted by "Kudu Jenkins (Code Review)" <ge...@cloudera.org>.
Kudu Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/24201 )
Change subject: [ingest] add C++ TSV loader and schema mapping
......................................................................
Patch Set 5:
Build Started http://jenkins.kudu.apache.org/job/pre_commit/2490/
--
To view, visit http://gerrit.cloudera.org:8080/24201
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I2d3fb6b031b26c4215de58de2162e060132846b4
Gerrit-Change-Number: 24201
Gerrit-PatchSet: 5
Gerrit-Owner: Zoltan Chovan <zc...@cloudera.com>
Gerrit-Reviewer: Attila Bukor <ab...@apache.org>
Gerrit-Reviewer: Gabriella Lotz <lo...@gmail.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Marton Greber <gr...@gmail.com>
Gerrit-Reviewer: Zoltan Chovan <zc...@cloudera.com>
Gerrit-Reviewer: Zoltan Martonka <zm...@cloudera.com>
Gerrit-Comment-Date: Mon, 20 Apr 2026 11:50:50 +0000
Gerrit-HasComments: No
[kudu-CR] [ingest] add C++ TSV loader and schema mapping
Posted by "Kudu Jenkins (Code Review)" <ge...@cloudera.org>.
Kudu Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/24201 )
Change subject: [ingest] add C++ TSV loader and schema mapping
......................................................................
Patch Set 8:
Build Started http://jenkins.kudu.apache.org/job/pre_commit/2534/
--
To view, visit http://gerrit.cloudera.org:8080/24201
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I2d3fb6b031b26c4215de58de2162e060132846b4
Gerrit-Change-Number: 24201
Gerrit-PatchSet: 8
Gerrit-Owner: Zoltan Chovan <zc...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <al...@apache.org>
Gerrit-Reviewer: Anonymous Coward (763)
Gerrit-Reviewer: Attila Bukor <ab...@apache.org>
Gerrit-Reviewer: Gabriella Lotz <lo...@gmail.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Marton Greber <gr...@gmail.com>
Gerrit-Reviewer: Zoltan Chovan <zc...@cloudera.com>
Gerrit-Reviewer: Zoltan Martonka <zm...@cloudera.com>
Gerrit-Comment-Date: Mon, 04 May 2026 10:46:28 +0000
Gerrit-HasComments: No
[kudu-CR] [ingest] add C++ TSV loader and schema mapping
Posted by "Kudu Jenkins (Code Review)" <ge...@cloudera.org>.
Kudu Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/24201 )
Change subject: [ingest] add C++ TSV loader and schema mapping
......................................................................
Patch Set 6:
Build Started http://jenkins.kudu.apache.org/job/pre_commit/2507/
--
To view, visit http://gerrit.cloudera.org:8080/24201
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I2d3fb6b031b26c4215de58de2162e060132846b4
Gerrit-Change-Number: 24201
Gerrit-PatchSet: 6
Gerrit-Owner: Zoltan Chovan <zc...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <al...@apache.org>
Gerrit-Reviewer: Attila Bukor <ab...@apache.org>
Gerrit-Reviewer: Gabriella Lotz <lo...@gmail.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Marton Greber <gr...@gmail.com>
Gerrit-Reviewer: Zoltan Chovan <zc...@cloudera.com>
Gerrit-Reviewer: Zoltan Martonka <zm...@cloudera.com>
Gerrit-Comment-Date: Wed, 22 Apr 2026 13:44:31 +0000
Gerrit-HasComments: No
[kudu-CR] [ingest] add C++ TSV loader and schema mapping
Posted by "Kudu Jenkins (Code Review)" <ge...@cloudera.org>.
Kudu Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/24201 )
Change subject: [ingest] add C++ TSV loader and schema mapping
......................................................................
Patch Set 1: Verified-1
Build Failed
http://jenkins.kudu.apache.org/job/pre_commit/2483/ : FAILURE
--
To view, visit http://gerrit.cloudera.org:8080/24201
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I2d3fb6b031b26c4215de58de2162e060132846b4
Gerrit-Change-Number: 24201
Gerrit-PatchSet: 1
Gerrit-Owner: Zoltan Chovan <zc...@cloudera.com>
Gerrit-Reviewer: Attila Bukor <ab...@apache.org>
Gerrit-Reviewer: Gabriella Lotz <lo...@gmail.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Marton Greber <gr...@gmail.com>
Gerrit-Reviewer: Zoltan Chovan <zc...@cloudera.com>
Gerrit-Reviewer: Zoltan Martonka <zm...@cloudera.com>
Gerrit-Comment-Date: Fri, 17 Apr 2026 15:21:21 +0000
Gerrit-HasComments: No
[kudu-CR] [ingest] add C++ TSV loader and schema mapping
Posted by "Alexey Serbin (Code Review)" <ge...@cloudera.org>.
Alexey Serbin has posted comments on this change. ( http://gerrit.cloudera.org:8080/24201 )
Change subject: [ingest] add C++ TSV loader and schema mapping
......................................................................
Patch Set 6:
(12 comments)
http://gerrit.cloudera.org:8080/#/c/24201/5/examples/ingest/cpp/README.md
File examples/ingest/cpp/README.md:
http://gerrit.cloudera.org:8080/#/c/24201/5/examples/ingest/cpp/README.md@10
PS5, Line 10: Batch inserts with `AUTO_FLUSH_BACKGROUND` session mode
> I wanted to benchmark tuning the batch sizes against auto flush, but if the
I don't any references on the benchmarks at this time, but it's quite simple to run a benchmark with `kudu perf loadgen` -- at least that's what I used for benchmarking a long time ago.
You can run `kudu perf loadgen` on your own any time either against an auto-generated table or a pre-created table with a particular schema.
`kudu perf loadgen` supports both AUTO_FLUSH_BACKGROUND and MANUAL_FLUSH mode: setting `--flush_per_n_rows` to anything greater than 0 switches it into the manual node. Also, it's possible to control the session buffer's size via the `--buffer_size_bytes` flag. There are also tunables to run multiple number of client threads, etc.:
https://kudu.apache.org/docs/command_line_tools_reference.html#perf-loadgen
http://gerrit.cloudera.org:8080/#/c/24201/6/examples/ingest/cpp/column_spec.h
File examples/ingest/cpp/column_spec.h:
PS6:
Why to separate this into its own file instead of keeping it as a part of kudu_insert.cc?
http://gerrit.cloudera.org:8080/#/c/24201/6/examples/ingest/cpp/kudu_insert.cc
File examples/ingest/cpp/kudu_insert.cc:
http://gerrit.cloudera.org:8080/#/c/24201/6/examples/ingest/cpp/kudu_insert.cc@20
PS6, Line 20: #include <getopt.h>
style nit: move system-wide C includes (getopt.h and unistd.h) to come first before the C++ ones in its own group
http://gerrit.cloudera.org:8080/#/c/24201/6/examples/ingest/cpp/kudu_insert.cc@102
PS6, Line 102: bool&
style nit: use pointer pass 'out' parameters to be consistent with the rest of Kudu C++ codebase
http://gerrit.cloudera.org:8080/#/c/24201/6/examples/ingest/cpp/kudu_insert.cc@142
PS6, Line 142: Status ParseInt64(const string_view str, int64_t* out) {
Any chance to templatize all ParseXxx to reduce boiler plate? I'd think it's possible because std::from_chars() is a function template.
http://gerrit.cloudera.org:8080/#/c/24201/6/examples/ingest/cpp/kudu_insert.cc@167
PS6, Line 167: int32_t parsed = 0;
: KUDU_RETURN_NOT_OK(ParseInt32(str, &parsed));
: if (parsed < numeric_limits<int16_t>::min() ||
: parsed > numeric_limits<int16_t>::max()) {
: return Status::InvalidArgument("int16 out of range: " + string(str));
: }
: *out = static_cast<int16_t>(parsed);
I'd think that std::from_chars() function template worked as expected for int16_t as well, wouldn't it?
http://gerrit.cloudera.org:8080/#/c/24201/6/examples/ingest/cpp/kudu_insert.cc@179
PS6, Line 179: vector<string_view> result;
nit: reserve/pre-allocate for the expected number of elements to avoid memory re-allocation and copying?
http://gerrit.cloudera.org:8080/#/c/24201/6/examples/ingest/cpp/kudu_insert.cc@201
PS6, Line 201: case ingest_schema::FieldType::kString:
: return KuduColumnSchema::STRING;
nit: maybe, move this under the 'default' case and remove extra 'return KuduColumnSchema::STRING'?
http://gerrit.cloudera.org:8080/#/c/24201/6/examples/ingest/cpp/kudu_insert.cc@262
PS6, Line 262: "CounterID", "EventDate", "UserID", "EventTime", "WatchID"
Why not to source these from ingest_schema::kPrimaryKeyColumns for consistency?
http://gerrit.cloudera.org:8080/#/c/24201/6/examples/ingest/cpp/kudu_insert.cc@265
PS6, Line 265: if (spec.is_primary_key) {
: continue;
: }
: schema_builder.AddColumn(spec.name)->Type(ToKuduType(spec.field_type))->NotNull();
nit: since CreateSchema() isn't at hot path by any means, maybe create a local set to comprise all the names of primary key columns and check the name of the column here instead of this superficial 'is_primary_key' field?
Alternatively, create an array of indices of the primary columns in the ingest_schema::kColumns, and check the presence of the current index there.
The idea is to get rid of the 'is_primary_key' since it duplicates information already provided in ngest_schema::kPrimaryKeyColumns.
http://gerrit.cloudera.org:8080/#/c/24201/6/examples/ingest/cpp/kudu_insert.cc@483
PS6, Line 483: cout << "[OK] Data ingestion completed successfully!\n";
IIUC, this message will appear even if there were errors reading in the data, etc. Is it as intended?
http://gerrit.cloudera.org:8080/#/c/24201/6/examples/ingest/cpp/kudu_insert.cc@487
PS6, Line 487: return 0;
IIUC, this would return 0 (meaning success) even if there were errors along the path (e.g., reading the data, writing to Kudu, etc.). Is it as intended?
--
To view, visit http://gerrit.cloudera.org:8080/24201
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I2d3fb6b031b26c4215de58de2162e060132846b4
Gerrit-Change-Number: 24201
Gerrit-PatchSet: 6
Gerrit-Owner: Zoltan Chovan <zc...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <al...@apache.org>
Gerrit-Reviewer: Anonymous Coward (763)
Gerrit-Reviewer: Attila Bukor <ab...@apache.org>
Gerrit-Reviewer: Gabriella Lotz <lo...@gmail.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Marton Greber <gr...@gmail.com>
Gerrit-Reviewer: Zoltan Chovan <zc...@cloudera.com>
Gerrit-Reviewer: Zoltan Martonka <zm...@cloudera.com>
Gerrit-Comment-Date: Mon, 27 Apr 2026 23:26:25 +0000
Gerrit-HasComments: Yes
[kudu-CR] [ingest] add C++ TSV loader and schema mapping
Posted by "Zoltan Chovan (Code Review)" <ge...@cloudera.org>.
Hello Marton Greber, Zoltan Martonka, Alexey Serbin, Gabriella Lotz, Attila Bukor, Kudu Jenkins, Anonymous Coward (763),
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/24201
to look at the new patch set (#8).
Change subject: [ingest] add C++ TSV loader and schema mapping
......................................................................
[ingest] add C++ TSV loader and schema mapping
Add a standalone C++ ingest client, build configuration, and usage docs
for loading the ClickHouse hits TSV dataset into Kudu. This creates a
high-throughput native baseline that can be benchmarked and tuned as
the foundation for the ingest pipeline work.
Change-Id: I2d3fb6b031b26c4215de58de2162e060132846b4
---
A examples/ingest/cpp/.gitignore
A examples/ingest/cpp/CMakeLists.txt
A examples/ingest/cpp/README.md
A examples/ingest/cpp/column_spec.h
A examples/ingest/cpp/kudu_insert.cc
5 files changed, 1,287 insertions(+), 0 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/01/24201/8
--
To view, visit http://gerrit.cloudera.org:8080/24201
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I2d3fb6b031b26c4215de58de2162e060132846b4
Gerrit-Change-Number: 24201
Gerrit-PatchSet: 8
Gerrit-Owner: Zoltan Chovan <zc...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <al...@apache.org>
Gerrit-Reviewer: Anonymous Coward (763)
Gerrit-Reviewer: Attila Bukor <ab...@apache.org>
Gerrit-Reviewer: Gabriella Lotz <lo...@gmail.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Marton Greber <gr...@gmail.com>
Gerrit-Reviewer: Zoltan Chovan <zc...@cloudera.com>
Gerrit-Reviewer: Zoltan Martonka <zm...@cloudera.com>
[kudu-CR] [ingest] add C++ TSV loader and schema mapping
Posted by "Zoltan Chovan (Code Review)" <ge...@cloudera.org>.
Hello Marton Greber, Zoltan Martonka, Alexey Serbin, Gabriella Lotz, Attila Bukor, Kudu Jenkins, Anonymous Coward (763),
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/24201
to look at the new patch set (#7).
Change subject: [ingest] add C++ TSV loader and schema mapping
......................................................................
[ingest] add C++ TSV loader and schema mapping
Add a standalone C++ ingest client, build configuration, and usage docs
for loading the ClickHouse hits TSV dataset into Kudu. This creates a
high-throughput native baseline that can be benchmarked and tuned as
the foundation for the ingest pipeline work.
Change-Id: I2d3fb6b031b26c4215de58de2162e060132846b4
---
A examples/ingest/cpp/.gitignore
A examples/ingest/cpp/CMakeLists.txt
A examples/ingest/cpp/README.md
A examples/ingest/cpp/column_spec.h
A examples/ingest/cpp/kudu_insert.cc
5 files changed, 1,280 insertions(+), 0 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/01/24201/7
--
To view, visit http://gerrit.cloudera.org:8080/24201
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I2d3fb6b031b26c4215de58de2162e060132846b4
Gerrit-Change-Number: 24201
Gerrit-PatchSet: 7
Gerrit-Owner: Zoltan Chovan <zc...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <al...@apache.org>
Gerrit-Reviewer: Anonymous Coward (763)
Gerrit-Reviewer: Attila Bukor <ab...@apache.org>
Gerrit-Reviewer: Gabriella Lotz <lo...@gmail.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Marton Greber <gr...@gmail.com>
Gerrit-Reviewer: Zoltan Chovan <zc...@cloudera.com>
Gerrit-Reviewer: Zoltan Martonka <zm...@cloudera.com>
[kudu-CR] ingest: add C++ TSV loader and schema mapping
Posted by "Zoltan Chovan (Code Review)" <ge...@cloudera.org>.
Hello Kudu Jenkins,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/24201
to look at the new patch set (#2).
Change subject: ingest: add C++ TSV loader and schema mapping
......................................................................
ingest: add C++ TSV loader and schema mapping
Add a standalone C++ ingest client, build configuration, and usage docs
for loading the ClickHouse hits TSV dataset into Kudu.This creates a
high-throughput native baseline that can be benchmarked and tuned as the
foundation for the ingest pipeline work.
Change-Id: I2d3fb6b031b26c4215de58de2162e060132846b4
---
A examples/ingest/cpp/.gitignore
A examples/ingest/cpp/CMakeLists.txt
A examples/ingest/cpp/README.md
A examples/ingest/cpp/kudu_insert.cc
4 files changed, 1,092 insertions(+), 0 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/01/24201/2
--
To view, visit http://gerrit.cloudera.org:8080/24201
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I2d3fb6b031b26c4215de58de2162e060132846b4
Gerrit-Change-Number: 24201
Gerrit-PatchSet: 2
Gerrit-Owner: Zoltan Chovan <zc...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
[kudu-CR] [ingest] add C++ TSV loader and schema mapping
Posted by "Zoltan Chovan (Code Review)" <ge...@cloudera.org>.
Hello Marton Greber, Zoltan Martonka, Gabriella Lotz, Attila Bukor, Kudu Jenkins,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/24201
to look at the new patch set (#5).
Change subject: [ingest] add C++ TSV loader and schema mapping
......................................................................
[ingest] add C++ TSV loader and schema mapping
Add a standalone C++ ingest client, build configuration, and usage docs
for loading the ClickHouse hits TSV dataset into Kudu.This creates a
high-throughput native baseline that can be benchmarked and tuned as the
foundation for the ingest pipeline work.
Change-Id: I2d3fb6b031b26c4215de58de2162e060132846b4
---
A examples/ingest/cpp/.gitignore
A examples/ingest/cpp/CMakeLists.txt
A examples/ingest/cpp/README.md
A examples/ingest/cpp/kudu_insert.cc
4 files changed, 1,092 insertions(+), 0 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/01/24201/5
--
To view, visit http://gerrit.cloudera.org:8080/24201
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I2d3fb6b031b26c4215de58de2162e060132846b4
Gerrit-Change-Number: 24201
Gerrit-PatchSet: 5
Gerrit-Owner: Zoltan Chovan <zc...@cloudera.com>
Gerrit-Reviewer: Attila Bukor <ab...@apache.org>
Gerrit-Reviewer: Gabriella Lotz <lo...@gmail.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Marton Greber <gr...@gmail.com>
Gerrit-Reviewer: Zoltan Chovan <zc...@cloudera.com>
Gerrit-Reviewer: Zoltan Martonka <zm...@cloudera.com>
[kudu-CR] [ingest] add C++ TSV loader and schema mapping
Posted by "Kudu Jenkins (Code Review)" <ge...@cloudera.org>.
Kudu Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/24201 )
Change subject: [ingest] add C++ TSV loader and schema mapping
......................................................................
Patch Set 4:
Build Started http://jenkins.kudu.apache.org/job/pre_commit/2489/
--
To view, visit http://gerrit.cloudera.org:8080/24201
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I2d3fb6b031b26c4215de58de2162e060132846b4
Gerrit-Change-Number: 24201
Gerrit-PatchSet: 4
Gerrit-Owner: Zoltan Chovan <zc...@cloudera.com>
Gerrit-Reviewer: Attila Bukor <ab...@apache.org>
Gerrit-Reviewer: Gabriella Lotz <lo...@gmail.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Marton Greber <gr...@gmail.com>
Gerrit-Reviewer: Zoltan Chovan <zc...@cloudera.com>
Gerrit-Reviewer: Zoltan Martonka <zm...@cloudera.com>
Gerrit-Comment-Date: Mon, 20 Apr 2026 09:28:49 +0000
Gerrit-HasComments: No
[kudu-CR] [ingest] add C++ TSV loader and schema mapping
Posted by "Kudu Jenkins (Code Review)" <ge...@cloudera.org>.
Kudu Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/24201 )
Change subject: [ingest] add C++ TSV loader and schema mapping
......................................................................
Patch Set 6: Verified-1
Build Failed
http://jenkins.kudu.apache.org/job/pre_commit/2507/ : FAILURE
--
To view, visit http://gerrit.cloudera.org:8080/24201
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I2d3fb6b031b26c4215de58de2162e060132846b4
Gerrit-Change-Number: 24201
Gerrit-PatchSet: 6
Gerrit-Owner: Zoltan Chovan <zc...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <al...@apache.org>
Gerrit-Reviewer: Anonymous Coward (763)
Gerrit-Reviewer: Attila Bukor <ab...@apache.org>
Gerrit-Reviewer: Gabriella Lotz <lo...@gmail.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Marton Greber <gr...@gmail.com>
Gerrit-Reviewer: Zoltan Chovan <zc...@cloudera.com>
Gerrit-Reviewer: Zoltan Martonka <zm...@cloudera.com>
Gerrit-Comment-Date: Wed, 22 Apr 2026 15:05:05 +0000
Gerrit-HasComments: No
[kudu-CR] ingest: add C++ TSV loader and schema mapping
Posted by "Kudu Jenkins (Code Review)" <ge...@cloudera.org>.
Kudu Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/24201 )
Change subject: ingest: add C++ TSV loader and schema mapping
......................................................................
Patch Set 1:
Build Started http://jenkins.kudu.apache.org/job/pre_commit/2483/
--
To view, visit http://gerrit.cloudera.org:8080/24201
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I2d3fb6b031b26c4215de58de2162e060132846b4
Gerrit-Change-Number: 24201
Gerrit-PatchSet: 1
Gerrit-Owner: Zoltan Chovan <zc...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Comment-Date: Fri, 17 Apr 2026 14:32:26 +0000
Gerrit-HasComments: No
[kudu-CR] [ingest] add C++ TSV loader and schema mapping
Posted by "Zoltan Martonka (Code Review)" <ge...@cloudera.org>.
Zoltan Martonka has posted comments on this change. ( http://gerrit.cloudera.org:8080/24201 )
Change subject: [ingest] add C++ TSV loader and schema mapping
......................................................................
Patch Set 3:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/24201/3/examples/ingest/cpp/kudu_insert.cc
File examples/ingest/cpp/kudu_insert.cc:
http://gerrit.cloudera.org:8080/#/c/24201/3/examples/ingest/cpp/kudu_insert.cc@267
PS3, Line 267:
please remove spaces from empty lines
--
To view, visit http://gerrit.cloudera.org:8080/24201
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I2d3fb6b031b26c4215de58de2162e060132846b4
Gerrit-Change-Number: 24201
Gerrit-PatchSet: 3
Gerrit-Owner: Zoltan Chovan <zc...@cloudera.com>
Gerrit-Reviewer: Attila Bukor <ab...@apache.org>
Gerrit-Reviewer: Gabriella Lotz <lo...@gmail.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Marton Greber <gr...@gmail.com>
Gerrit-Reviewer: Zoltan Chovan <zc...@cloudera.com>
Gerrit-Reviewer: Zoltan Martonka <zm...@cloudera.com>
Gerrit-Comment-Date: Mon, 20 Apr 2026 08:56:00 +0000
Gerrit-HasComments: Yes
[kudu-CR] [ingest] add C++ TSV loader and schema mapping
Posted by "Zoltan Chovan (Code Review)" <ge...@cloudera.org>.
Zoltan Chovan has posted comments on this change. ( http://gerrit.cloudera.org:8080/24201 )
Change subject: [ingest] add C++ TSV loader and schema mapping
......................................................................
Patch Set 5:
(2 comments)
http://gerrit.cloudera.org:8080/#/c/24201/4/examples/ingest/cpp/kudu_insert.cc
File examples/ingest/cpp/kudu_insert.cc:
http://gerrit.cloudera.org:8080/#/c/24201/4/examples/ingest/cpp/kudu_insert.cc@454
PS4, Line 454: KUDU_RETURN_NOT_OK(row->SetString("FromTag", fields[100]));
> You should define a kudu_throw_not_ok macro, because you seem to try to cat
the row->SetX methods return Statuses, otherwise the try/catch is present for the ParseX methods, since they use std::stoll and std::stoi, which can throw std::invalid_argument or std::out_of_range exceptions
http://gerrit.cloudera.org:8080/#/c/24201/4/examples/ingest/cpp/kudu_insert.cc@565
PS4, Line 565: IngestResult ingest_result;
: s = InsertDataFromFile(client, config.file_path, &ingest_result);
: if (!s.ok()) {
: std::cerr << "Error inserting data: " << s.ToString() << std::endl;
: return 1;
: }
> Is the application returning 0 ok if some rows had parsing errors and was n
yes, if the parse fails within the ParseX() methods, then the exception is caught and logged, the error count increased, and the processing continues
in main() the returned non-OK statuses can cause return 1
--
To view, visit http://gerrit.cloudera.org:8080/24201
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I2d3fb6b031b26c4215de58de2162e060132846b4
Gerrit-Change-Number: 24201
Gerrit-PatchSet: 5
Gerrit-Owner: Zoltan Chovan <zc...@cloudera.com>
Gerrit-Reviewer: Attila Bukor <ab...@apache.org>
Gerrit-Reviewer: Gabriella Lotz <lo...@gmail.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Marton Greber <gr...@gmail.com>
Gerrit-Reviewer: Zoltan Chovan <zc...@cloudera.com>
Gerrit-Reviewer: Zoltan Martonka <zm...@cloudera.com>
Gerrit-Comment-Date: Mon, 20 Apr 2026 12:02:07 +0000
Gerrit-HasComments: Yes
[kudu-CR] [ingest] add C++ TSV loader and schema mapping
Posted by "Zoltan Chovan (Code Review)" <ge...@cloudera.org>.
Hello Kudu Jenkins,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/24201
to look at the new patch set (#3).
Change subject: [ingest] add C++ TSV loader and schema mapping
......................................................................
[ingest] add C++ TSV loader and schema mapping
Add a standalone C++ ingest client, build configuration, and usage docs
for loading the ClickHouse hits TSV dataset into Kudu.This creates a
high-throughput native baseline that can be benchmarked and tuned as the
foundation for the ingest pipeline work.
Change-Id: I2d3fb6b031b26c4215de58de2162e060132846b4
---
A examples/ingest/cpp/.gitignore
A examples/ingest/cpp/CMakeLists.txt
A examples/ingest/cpp/README.md
A examples/ingest/cpp/kudu_insert.cc
4 files changed, 1,092 insertions(+), 0 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/01/24201/3
--
To view, visit http://gerrit.cloudera.org:8080/24201
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I2d3fb6b031b26c4215de58de2162e060132846b4
Gerrit-Change-Number: 24201
Gerrit-PatchSet: 3
Gerrit-Owner: Zoltan Chovan <zc...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
[kudu-CR] [ingest] add C++ TSV loader and schema mapping
Posted by "Kudu Jenkins (Code Review)" <ge...@cloudera.org>.
Kudu Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/24201 )
Change subject: [ingest] add C++ TSV loader and schema mapping
......................................................................
Patch Set 7: Verified-1
Build Failed
http://jenkins.kudu.apache.org/job/pre_commit/2532/ : FAILURE
--
To view, visit http://gerrit.cloudera.org:8080/24201
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I2d3fb6b031b26c4215de58de2162e060132846b4
Gerrit-Change-Number: 24201
Gerrit-PatchSet: 7
Gerrit-Owner: Zoltan Chovan <zc...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <al...@apache.org>
Gerrit-Reviewer: Anonymous Coward (763)
Gerrit-Reviewer: Attila Bukor <ab...@apache.org>
Gerrit-Reviewer: Gabriella Lotz <lo...@gmail.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Marton Greber <gr...@gmail.com>
Gerrit-Reviewer: Zoltan Chovan <zc...@cloudera.com>
Gerrit-Reviewer: Zoltan Martonka <zm...@cloudera.com>
Gerrit-Comment-Date: Mon, 04 May 2026 08:31:06 +0000
Gerrit-HasComments: No
[kudu-CR] [ingest] add C++ TSV loader and schema mapping
Posted by "Kudu Jenkins (Code Review)" <ge...@cloudera.org>.
Kudu Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/24201 )
Change subject: [ingest] add C++ TSV loader and schema mapping
......................................................................
Patch Set 8: Verified-1
Build Failed
http://jenkins.kudu.apache.org/job/pre_commit/2534/ : FAILURE
--
To view, visit http://gerrit.cloudera.org:8080/24201
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I2d3fb6b031b26c4215de58de2162e060132846b4
Gerrit-Change-Number: 24201
Gerrit-PatchSet: 8
Gerrit-Owner: Zoltan Chovan <zc...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <al...@apache.org>
Gerrit-Reviewer: Anonymous Coward (763)
Gerrit-Reviewer: Attila Bukor <ab...@apache.org>
Gerrit-Reviewer: Gabriella Lotz <lo...@gmail.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Marton Greber <gr...@gmail.com>
Gerrit-Reviewer: Zoltan Chovan <zc...@cloudera.com>
Gerrit-Reviewer: Zoltan Martonka <zm...@cloudera.com>
Gerrit-Comment-Date: Mon, 04 May 2026 12:34:07 +0000
Gerrit-HasComments: No
[kudu-CR] [ingest] add C++ TSV loader and schema mapping
Posted by "Zoltan Chovan (Code Review)" <ge...@cloudera.org>.
Zoltan Chovan has posted comments on this change. ( http://gerrit.cloudera.org:8080/24201 )
Change subject: [ingest] add C++ TSV loader and schema mapping
......................................................................
Patch Set 7:
(11 comments)
http://gerrit.cloudera.org:8080/#/c/24201/6/examples/ingest/cpp/column_spec.h
File examples/ingest/cpp/column_spec.h:
PS6:
> Why to separate this into its own file instead of keeping it as a part of k
I wanted to improve kudu_insert.cc's readability, and having 105 lines to describe the schema was a bit ugly. This way the logic and the schema description is separate, and in my opinion a bit more easily readable.
http://gerrit.cloudera.org:8080/#/c/24201/6/examples/ingest/cpp/kudu_insert.cc
File examples/ingest/cpp/kudu_insert.cc:
http://gerrit.cloudera.org:8080/#/c/24201/6/examples/ingest/cpp/kudu_insert.cc@20
PS6, Line 20:
> style nit: move system-wide C includes (getopt.h and unistd.h) to come firs
Done
http://gerrit.cloudera.org:8080/#/c/24201/6/examples/ingest/cpp/kudu_insert.cc@102
PS6, Line 102: Defau
> style nit: use pointer pass 'out' parameters to be consistent with the rest
Done
http://gerrit.cloudera.org:8080/#/c/24201/6/examples/ingest/cpp/kudu_insert.cc@142
PS6, Line 142: return true;
> Any chance to templatize all ParseXxx to reduce boiler plate? I'd think it
Done
http://gerrit.cloudera.org:8080/#/c/24201/6/examples/ingest/cpp/kudu_insert.cc@167
PS6, Line 167: string_view TrimAsciiWhitespace(const string_view input) {
: const auto is_ascii_whitespace = [](const char ch) {
: return ch == ' ' || ch == '\t' || ch == '\n' || ch == '\r';
: };
:
: size_t begin_idx = 0;
: size_t end_idx = input.size();
> I'd think that std::from_chars() function template worked as expected for i
Done
http://gerrit.cloudera.org:8080/#/c/24201/6/examples/ingest/cpp/kudu_insert.cc@179
PS6, Line 179: }
> nit: reserve/pre-allocate for the expected number of elements to avoid memo
Done
http://gerrit.cloudera.org:8080/#/c/24201/6/examples/ingest/cpp/kudu_insert.cc@201
PS6, Line 201: size_t start = 0;
: while (start <= line.size()) {
> nit: maybe, move this under the 'default' case and remove extra 'return Kud
Done
http://gerrit.cloudera.org:8080/#/c/24201/6/examples/ingest/cpp/kudu_insert.cc@262
PS6, Line 262: dx < kExpectedFieldCount; ++tsv_idx) {
> Why not to source these from ingest_schema::kPrimaryKeyColumns for consiste
Done
http://gerrit.cloudera.org:8080/#/c/24201/6/examples/ingest/cpp/kudu_insert.cc@265
PS6, Line 265: "missing column in table schema: " + string(ingest_schema::kColumns[tsv_idx].name));
: }
: }
:
> nit: since CreateSchema() isn't at hot path by any means, maybe create a lo
right, I changed the primary-key lookup as suggested, the logic is in IsPrimaryKeyColumnName() now, and the is_primary_key field is now removed
http://gerrit.cloudera.org:8080/#/c/24201/6/examples/ingest/cpp/kudu_insert.cc@483
PS6, Line 483:
> IIUC, this message will appear even if there were errors reading in the dat
Done
http://gerrit.cloudera.org:8080/#/c/24201/6/examples/ingest/cpp/kudu_insert.cc@487
PS6, Line 487: cout <<
> IIUC, this would return 0 (meaning success) even if there were errors along
Done
--
To view, visit http://gerrit.cloudera.org:8080/24201
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I2d3fb6b031b26c4215de58de2162e060132846b4
Gerrit-Change-Number: 24201
Gerrit-PatchSet: 7
Gerrit-Owner: Zoltan Chovan <zc...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <al...@apache.org>
Gerrit-Reviewer: Anonymous Coward (763)
Gerrit-Reviewer: Attila Bukor <ab...@apache.org>
Gerrit-Reviewer: Gabriella Lotz <lo...@gmail.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Marton Greber <gr...@gmail.com>
Gerrit-Reviewer: Zoltan Chovan <zc...@cloudera.com>
Gerrit-Reviewer: Zoltan Martonka <zm...@cloudera.com>
Gerrit-Comment-Date: Mon, 04 May 2026 07:16:19 +0000
Gerrit-HasComments: Yes
[kudu-CR] [ingest] add C++ TSV loader and schema mapping
Posted by "Kudu Jenkins (Code Review)" <ge...@cloudera.org>.
Kudu Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/24201 )
Change subject: [ingest] add C++ TSV loader and schema mapping
......................................................................
Patch Set 7:
Build Started http://jenkins.kudu.apache.org/job/pre_commit/2532/
--
To view, visit http://gerrit.cloudera.org:8080/24201
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I2d3fb6b031b26c4215de58de2162e060132846b4
Gerrit-Change-Number: 24201
Gerrit-PatchSet: 7
Gerrit-Owner: Zoltan Chovan <zc...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <al...@apache.org>
Gerrit-Reviewer: Anonymous Coward (763)
Gerrit-Reviewer: Attila Bukor <ab...@apache.org>
Gerrit-Reviewer: Gabriella Lotz <lo...@gmail.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Marton Greber <gr...@gmail.com>
Gerrit-Reviewer: Zoltan Chovan <zc...@cloudera.com>
Gerrit-Reviewer: Zoltan Martonka <zm...@cloudera.com>
Gerrit-Comment-Date: Mon, 04 May 2026 07:16:02 +0000
Gerrit-HasComments: No
[kudu-CR] [ingest] add C++ TSV loader and schema mapping
Posted by "Kudu Jenkins (Code Review)" <ge...@cloudera.org>.
Kudu Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/24201 )
Change subject: [ingest] add C++ TSV loader and schema mapping
......................................................................
Patch Set 5: Verified-1
Build Failed
http://jenkins.kudu.apache.org/job/pre_commit/2490/ : FAILURE
--
To view, visit http://gerrit.cloudera.org:8080/24201
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I2d3fb6b031b26c4215de58de2162e060132846b4
Gerrit-Change-Number: 24201
Gerrit-PatchSet: 5
Gerrit-Owner: Zoltan Chovan <zc...@cloudera.com>
Gerrit-Reviewer: Attila Bukor <ab...@apache.org>
Gerrit-Reviewer: Gabriella Lotz <lo...@gmail.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Marton Greber <gr...@gmail.com>
Gerrit-Reviewer: Zoltan Chovan <zc...@cloudera.com>
Gerrit-Reviewer: Zoltan Martonka <zm...@cloudera.com>
Gerrit-Comment-Date: Mon, 20 Apr 2026 12:36:45 +0000
Gerrit-HasComments: No
[kudu-CR] [ingest] add C++ TSV loader and schema mapping
Posted by "Zoltan Martonka (Code Review)" <ge...@cloudera.org>.
Zoltan Martonka has posted comments on this change. ( http://gerrit.cloudera.org:8080/24201 )
Change subject: [ingest] add C++ TSV loader and schema mapping
......................................................................
Patch Set 5: Code-Review+1
--
To view, visit http://gerrit.cloudera.org:8080/24201
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I2d3fb6b031b26c4215de58de2162e060132846b4
Gerrit-Change-Number: 24201
Gerrit-PatchSet: 5
Gerrit-Owner: Zoltan Chovan <zc...@cloudera.com>
Gerrit-Reviewer: Attila Bukor <ab...@apache.org>
Gerrit-Reviewer: Gabriella Lotz <lo...@gmail.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Marton Greber <gr...@gmail.com>
Gerrit-Reviewer: Zoltan Chovan <zc...@cloudera.com>
Gerrit-Reviewer: Zoltan Martonka <zm...@cloudera.com>
Gerrit-Comment-Date: Mon, 20 Apr 2026 12:12:29 +0000
Gerrit-HasComments: No
[kudu-CR] [ingest] add C++ TSV loader and schema mapping
Posted by "Zoltan Chovan (Code Review)" <ge...@cloudera.org>.
Hello Marton Greber, Zoltan Martonka, Gabriella Lotz, Attila Bukor, Kudu Jenkins,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/24201
to look at the new patch set (#4).
Change subject: [ingest] add C++ TSV loader and schema mapping
......................................................................
[ingest] add C++ TSV loader and schema mapping
Add a standalone C++ ingest client, build configuration, and usage docs
for loading the ClickHouse hits TSV dataset into Kudu.This creates a
high-throughput native baseline that can be benchmarked and tuned as the
foundation for the ingest pipeline work.
Change-Id: I2d3fb6b031b26c4215de58de2162e060132846b4
---
A examples/ingest/cpp/.gitignore
A examples/ingest/cpp/CMakeLists.txt
A examples/ingest/cpp/README.md
A examples/ingest/cpp/kudu_insert.cc
4 files changed, 1,092 insertions(+), 0 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/01/24201/4
--
To view, visit http://gerrit.cloudera.org:8080/24201
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I2d3fb6b031b26c4215de58de2162e060132846b4
Gerrit-Change-Number: 24201
Gerrit-PatchSet: 4
Gerrit-Owner: Zoltan Chovan <zc...@cloudera.com>
Gerrit-Reviewer: Attila Bukor <ab...@apache.org>
Gerrit-Reviewer: Gabriella Lotz <lo...@gmail.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Marton Greber <gr...@gmail.com>
Gerrit-Reviewer: Zoltan Chovan <zc...@cloudera.com>
Gerrit-Reviewer: Zoltan Martonka <zm...@cloudera.com>
[kudu-CR] [ingest] add C++ TSV loader and schema mapping
Posted by "Alexey Serbin (Code Review)" <ge...@cloudera.org>.
Alexey Serbin has posted comments on this change. ( http://gerrit.cloudera.org:8080/24201 )
Change subject: [ingest] add C++ TSV loader and schema mapping
......................................................................
Patch Set 5:
(13 comments)
http://gerrit.cloudera.org:8080/#/c/24201/5/examples/ingest/cpp/README.md
File examples/ingest/cpp/README.md:
http://gerrit.cloudera.org:8080/#/c/24201/5/examples/ingest/cpp/README.md@10
PS5, Line 10: Batch inserts with manual flush control for high performance
I'm curious: what amount of performance benefits do you find in using manual flashing compared with auto-flush background?
Per my knowledge, AUTO_FLUSH_BACKGROUND in the C++ client has shown better results using the same buffer size compared with manual (and, of course, and so-called auto-flush).
http://gerrit.cloudera.org:8080/#/c/24201/5/examples/ingest/cpp/kudu_insert.cc
File examples/ingest/cpp/kudu_insert.cc:
http://gerrit.cloudera.org:8080/#/c/24201/5/examples/ingest/cpp/kudu_insert.cc@51
PS5, Line 51: std::
Why not to add using directives for STL classes similar to what's done for Kudu class and get rid of these 'std::' prefixes?
http://gerrit.cloudera.org:8080/#/c/24201/5/examples/ingest/cpp/kudu_insert.cc@71
PS5, Line 71: [master_address]"
: << " [--master <host:port>]
This is confusing: how to provide master's end-point address? Is it from a positional argument of from command-line flag?
http://gerrit.cloudera.org:8080/#/c/24201/5/examples/ingest/cpp/kudu_insert.cc@84
PS5, Line 84: config->master_address = GetEnvOrDefault(kMasterEnvVar, "localhost:7051");
This looks strange and confusing:
1) there is env var for master address
2) there is positional arg
3) there is --master flag
Which one to use? And why to have that many options at all?
Also, if env variable, if set, usually overrides command-line options. Here it's a reversed convention, which adds extra confusion.
http://gerrit.cloudera.org:8080/#/c/24201/5/examples/ingest/cpp/kudu_insert.cc@86
PS5, Line 86: bool master_set = false;
: for (int i = 2; i < argc; ++i) {
: std::string arg = argv[i];
: if (arg == "--help" || arg == "-h") {
: return false;
: }
: if (arg == "--master" && i + 1 < argc) {
: config->master_address = argv[++i];
: master_set = true;
: continue;
: }
: if (arg.rfind("--master=", 0) == 0) {
: config->master_address = arg.substr(std::strlen("--master="));
: master_set = true;
: continue;
: }
: if (arg.rfind("--", 0) != 0 && !master_set) {
: config->master_address = arg;
: master_set = true;
: continue;
: }
: return false;
: }
Any chance of using getopt()/getopt_long() or maybe something from the Boost instead of this?
http://gerrit.cloudera.org:8080/#/c/24201/5/examples/ingest/cpp/kudu_insert.cc@114
PS5, Line 114: static int64_t ParseInt64(const std::string& str) {
: return std::stoll(str);
: }
:
: // Helper function to parse int32 from string
: static int32_t ParseInt32(const std::string& str) {
: return std::stoi(str);
: }
:
: // Helper function to parse int16 from string
: static int16_t ParseInt16(const std::string& str) {
: return static_cast<int16_t>(std::stoi(str));
: }
These str-to-int converters are quite lame. They not only accept invalid input and stay silent about that (e.g., try '100x' instead of '100'), but they aren't as performant as strtol().
I'd consider using strto32_adapter() and similar from the Kudu's codebase instead.
http://gerrit.cloudera.org:8080/#/c/24201/5/examples/ingest/cpp/kudu_insert.cc@129
PS5, Line 129: std::string
Consider using std::string_view instead if the source string stays in scope. It helps avoiding useless memory allocations and copying.
http://gerrit.cloudera.org:8080/#/c/24201/5/examples/ingest/cpp/kudu_insert.cc@269
PS5, Line 269: std::endl
If you call this app "high-performant", you may consider stop using std::endl altogether for non-cerr streams: https://clang.llvm.org/extra/clang-tidy/checks/performance/avoid-endl.html
http://gerrit.cloudera.org:8080/#/c/24201/5/examples/ingest/cpp/kudu_insert.cc@289
PS5, Line 289: if (!s.ok()) {
: return s;
: }
Why not to use KUDU_RETURN_NOT_OK here and elsewhere instead of this boilerplate?
http://gerrit.cloudera.org:8080/#/c/24201/5/examples/ingest/cpp/kudu_insert.cc@337
PS5, Line 337: std::string
Use std::string_view here instead to useless avoid memory allocation and copying?
http://gerrit.cloudera.org:8080/#/c/24201/5/examples/ingest/cpp/kudu_insert.cc@348
PS5, Line 348:
What exceptions do you expect to catch here? Kudu API isn't supposed to throw anything. Consider moving this try/catch to the place where you expect something to be thrown.
http://gerrit.cloudera.org:8080/#/c/24201/5/examples/ingest/cpp/kudu_insert.cc@354
PS5, Line 354: KUDU_RETURN_NOT_OK(row->SetInt64("WatchID", ParseInt64(fields[0])));
: KUDU_RETURN_NOT_OK(row->SetInt16("JavaEnable", ParseInt16(fields[1])));
: KUDU_RETURN_NOT_OK(row->SetString("Title", fields[2]));
: KUDU_RETURN_NOT_OK(row->SetInt16("GoodEvent", ParseInt16(fields[3])));
: KUDU_RETURN_NOT_OK(row->SetUnixTimeMicros("EventTime", ParseInt64(fields[4])));
: KUDU_RETURN_NOT_OK(row->SetInt32("EventDate", ParseInt32(fields[5])));
: KUDU_RETURN_NOT_OK(row->SetInt32("CounterID", ParseInt32(fields[6])));
: KUDU_RETURN_NOT_OK(row->SetInt32("ClientIP", ParseInt32(fields[7])));
: KUDU_RETURN_NOT_OK(row->SetInt32("RegionID", ParseInt32(fields[8])));
: KUDU_RETURN_NOT_OK(row->SetInt64("UserID", ParseInt64(fields[9])));
: KUDU_RETURN_NOT_OK(row->SetInt16("CounterClass", ParseInt16(fields[10])));
: KUDU_RETURN_NOT_OK(row->SetInt16("OS", ParseInt16(fields[11])));
: KUDU_RETURN_NOT_OK(row->SetInt16("UserAgent", ParseInt16(fields[12])));
: KUDU_RETURN_NOT_OK(row->SetString("URL", fields[13]));
: KUDU_RETURN_NOT_OK(row->SetString("Referer", fields[14]));
: KUDU_RETURN_NOT_OK(row->SetInt16("IsRefresh", ParseInt16(fields[15])));
: KUDU_RETURN_NOT_OK(row->SetInt16("RefererCategoryID", ParseInt16(fields[16])));
: KUDU_RETURN_NOT_OK(row->SetInt32("RefererRegionID", ParseInt32(fields[17])));
: KUDU_RETURN_NOT_OK(row->SetInt16("URLCategoryID", ParseInt16(fields[18])));
: KUDU_RETURN_NOT_OK(row->SetInt32("URLRegionID", ParseInt32(fields[19])));
: KUDU_RETURN_NOT_OK(row->SetInt16("ResolutionWidth", ParseInt16(fields[20])));
: KUDU_RETURN_NOT_OK(row->SetInt16("ResolutionHeight", ParseInt16(fields[21])));
: KUDU_RETURN_NOT_OK(row->SetInt16("ResolutionDepth", ParseInt16(fields[22])));
: KUDU_RETURN_NOT_OK(row->SetInt16("FlashMajor", ParseInt16(fields[23])));
: KUDU_RETURN_NOT_OK(row->SetInt16("FlashMinor", ParseInt16(fields[24])));
: KUDU_RETURN_NOT_OK(row->SetString("FlashMinor2", fields[25]));
: KUDU_RETURN_NOT_OK(row->SetInt16("NetMajor", ParseInt16(fields[26])));
: KUDU_RETURN_NOT_OK(row->SetInt16("NetMinor", ParseInt16(fields[27])));
: KUDU_RETURN_NOT_OK(row->SetInt16("UserAgentMajor", ParseInt16(fields[28])));
: KUDU_RETURN_NOT_OK(row->SetString("UserAgentMinor", fields[29]));
: KUDU_RETURN_NOT_OK(row->SetInt16("CookieEnable", ParseInt16(fields[30])));
: KUDU_RETURN_NOT_OK(row->SetInt16("JavascriptEnable", ParseInt16(fields[31])));
: KUDU_RETURN_NOT_OK(row->SetInt16("IsMobile", ParseInt16(fields[32])));
: KUDU_RETURN_NOT_OK(row->SetInt16("MobilePhone", ParseInt16(fields[33])));
: KUDU_RETURN_NOT_OK(row->SetString("MobilePhoneModel", fields[34]));
: KUDU_RETURN_NOT_OK(row->SetString("Params", fields[35]));
: KUDU_RETURN_NOT_OK(row->SetInt32("IPNetworkID", ParseInt32(fields[36])));
: KUDU_RETURN_NOT_OK(row->SetInt16("TraficSourceID", ParseInt16(fields[37])));
: KUDU_RETURN_NOT_OK(row->SetInt16("SearchEngineID", ParseInt16(fields[38])));
: KUDU_RETURN_NOT_OK(row->SetString("SearchPhrase", fields[39]));
: KUDU_RETURN_NOT_OK(row->SetInt16("AdvEngineID", ParseInt16(fields[40])));
: KUDU_RETURN_NOT_OK(row->SetInt16("IsArtifical", ParseInt16(fields[41])));
: KUDU_RETURN_NOT_OK(row->SetInt16("WindowClientWidth", ParseInt16(fields[42])));
: KUDU_RETURN_NOT_OK(row->SetInt16("WindowClientHeight", ParseInt16(fields[43])));
: KUDU_RETURN_NOT_OK(row->SetInt16("ClientTimeZone", ParseInt16(fields[44])));
: KUDU_RETURN_NOT_OK(row->SetUnixTimeMicros("ClientEventTime", ParseInt64(fields[45])));
: KUDU_RETURN_NOT_OK(row->SetInt16("SilverlightVersion1", ParseInt16(fields[46])));
: KUDU_RETURN_NOT_OK(row->SetInt16("SilverlightVersion2", ParseInt16(fields[47])));
: KUDU_RETURN_NOT_OK(row->SetInt32("SilverlightVersion3", ParseInt32(fields[48])));
: KUDU_RETURN_NOT_OK(row->SetInt16("SilverlightVersion4", ParseInt16(fields[49])));
: KUDU_RETURN_NOT_OK(row->SetString("PageCharset", fields[50]));
: KUDU_RETURN_NOT_OK(row->SetInt32("CodeVersion", ParseInt32(fields[51])));
: KUDU_RETURN_NOT_OK(row->SetInt16("IsLink", ParseInt16(fields[52])));
: KUDU_RETURN_NOT_OK(row->SetInt16("IsDownload", ParseInt16(fields[53])));
: KUDU_RETURN_NOT_OK(row->SetInt16("IsNotBounce", ParseInt16(fields[54])));
: KUDU_RETURN_NOT_OK(row->SetInt64("FUniqID", ParseInt64(fields[55])));
: KUDU_RETURN_NOT_OK(row->SetString("OriginalURL", fields[56]));
: KUDU_RETURN_NOT_OK(row->SetInt32("HID", ParseInt32(fields[57])));
: KUDU_RETURN_NOT_OK(row->SetInt16("IsOldCounter", ParseInt16(fields[58])));
: KUDU_RETURN_NOT_OK(row->SetInt16("IsEvent", ParseInt16(fields[59])));
: KUDU_RETURN_NOT_OK(row->SetInt16("IsParameter", ParseInt16(fields[60])));
: KUDU_RETURN_NOT_OK(row->SetInt16("DontCountHits", ParseInt16(fields[61])));
: KUDU_RETURN_NOT_OK(row->SetInt16("WithHash", ParseInt16(fields[62])));
: KUDU_RETURN_NOT_OK(row->SetString("HitColor", fields[63]));
: KUDU_RETURN_NOT_OK(row->SetUnixTimeMicros("LocalEventTime", ParseInt64(fields[64])));
: KUDU_RETURN_NOT_OK(row->SetInt16("Age", ParseInt16(fields[65])));
: KUDU_RETURN_NOT_OK(row->SetInt16("Sex", ParseInt16(fields[66])));
: KUDU_RETURN_NOT_OK(row->SetInt16("Income", ParseInt16(fields[67])));
: KUDU_RETURN_NOT_OK(row->SetInt16("Interests", ParseInt16(fields[68])));
: KUDU_RETURN_NOT_OK(row->SetInt16("Robotness", ParseInt16(fields[69])));
: KUDU_RETURN_NOT_OK(row->SetInt32("RemoteIP", ParseInt32(fields[70])));
: KUDU_RETURN_NOT_OK(row->SetInt32("WindowName", ParseInt32(fields[71])));
: KUDU_RETURN_NOT_OK(row->SetInt32("OpenerName", ParseInt32(fields[72])));
: KUDU_RETURN_NOT_OK(row->SetInt16("HistoryLength", ParseInt16(fields[73])));
: KUDU_RETURN_NOT_OK(row->SetString("BrowserLanguage", fields[74]));
: KUDU_RETURN_NOT_OK(row->SetString("BrowserCountry", fields[75]));
: KUDU_RETURN_NOT_OK(row->SetString("SocialNetwork", fields[76]));
: KUDU_RETURN_NOT_OK(row->SetString("SocialAction", fields[77]));
: KUDU_RETURN_NOT_OK(row->SetInt16("HTTPError", ParseInt16(fields[78])));
: KUDU_RETURN_NOT_OK(row->SetInt32("SendTiming", ParseInt32(fields[79])));
: KUDU_RETURN_NOT_OK(row->SetInt32("DNSTiming", ParseInt32(fields[80])));
: KUDU_RETURN_NOT_OK(row->SetInt32("ConnectTiming", ParseInt32(fields[81])));
: KUDU_RETURN_NOT_OK(row->SetInt32("ResponseStartTiming", ParseInt32(fields[82])));
: KUDU_RETURN_NOT_OK(row->SetInt32("ResponseEndTiming", ParseInt32(fields[83])));
: KUDU_RETURN_NOT_OK(row->SetInt32("FetchTiming", ParseInt32(fields[84])));
: KUDU_RETURN_NOT_OK(row->SetInt16("SocialSourceNetworkID", ParseInt16(fields[85])));
: KUDU_RETURN_NOT_OK(row->SetString("SocialSourcePage", fields[86]));
: KUDU_RETURN_NOT_OK(row->SetInt64("ParamPrice", ParseInt64(fields[87])));
: KUDU_RETURN_NOT_OK(row->SetString("ParamOrderID", fields[88]));
: KUDU_RETURN_NOT_OK(row->SetString("ParamCurrency", fields[89]));
: KUDU_RETURN_NOT_OK(row->SetInt16("ParamCurrencyID", ParseInt16(fields[90])));
: KUDU_RETURN_NOT_OK(row->SetString("OpenstatServiceName", fields[91]));
: KUDU_RETURN_NOT_OK(row->SetString("OpenstatCampaignID", fields[92]));
: KUDU_RETURN_NOT_OK(row->SetString("OpenstatAdID", fields[93]));
: KUDU_RETURN_NOT_OK(row->SetString("OpenstatSourceID", fields[94]));
: KUDU_RETURN_NOT_OK(row->SetString("UTMSource", fields[95]));
: KUDU_RETURN_NOT_OK(row->SetString("UTMMedium", fields[96]));
: KUDU_RETURN_NOT_OK(row->SetString("UTMCampaign", fields[97]));
: KUDU_RETURN_NOT_OK(row->SetString("UTMContent", fields[98]));
: KUDU_RETURN_NOT_OK(row->SetString("UTMTerm", fields[99]));
: KUDU_RETURN_NOT_OK(row->SetString("FromTag", fields[100]));
: KUDU_RETURN_NOT_OK(row->SetInt16("HasGCLID", ParseInt16(fields[101])));
: KUDU_RETURN_NOT_OK(row->SetInt64("RefererHash", ParseInt64(fields[102])));
: KUDU_RETURN_NOT_OK(row->SetInt64("URLHash", ParseInt64(fields[103])));
: KUDU_RETURN_NOT_OK(row->SetInt32("CLID", ParseInt32(fields[104])));
Separate this ugly blob this into its function, maybe?
Also, create an array with the names of the columns, and address those by the index, similar to how the fields are addressed. Re-use the newly introduced array for table schema's creation.
http://gerrit.cloudera.org:8080/#/c/24201/5/examples/ingest/cpp/kudu_insert.cc@470
PS5, Line 470: // Periodic flush for better performance
: if (rows_inserted % kFlushInterval == 0) {
: KUDU_RETURN_NOT_OK(session->Flush());
: std::cout << "Processed " << rows_inserted << " rows..." << std::endl;
: }
Why not to use AUTO_FLUSH_BACKGROUND instead? With particular settings for the buffer space, this approach will bomb out because of running out of write buffer.
--
To view, visit http://gerrit.cloudera.org:8080/24201
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I2d3fb6b031b26c4215de58de2162e060132846b4
Gerrit-Change-Number: 24201
Gerrit-PatchSet: 5
Gerrit-Owner: Zoltan Chovan <zc...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <al...@apache.org>
Gerrit-Reviewer: Attila Bukor <ab...@apache.org>
Gerrit-Reviewer: Gabriella Lotz <lo...@gmail.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Marton Greber <gr...@gmail.com>
Gerrit-Reviewer: Zoltan Chovan <zc...@cloudera.com>
Gerrit-Reviewer: Zoltan Martonka <zm...@cloudera.com>
Gerrit-Comment-Date: Mon, 20 Apr 2026 20:49:11 +0000
Gerrit-HasComments: Yes
[kudu-CR] [ingest] add C++ TSV loader and schema mapping
Posted by "Zoltan Chovan (Code Review)" <ge...@cloudera.org>.
Zoltan Chovan has posted comments on this change. ( http://gerrit.cloudera.org:8080/24201 )
Change subject: [ingest] add C++ TSV loader and schema mapping
......................................................................
Patch Set 6:
(13 comments)
http://gerrit.cloudera.org:8080/#/c/24201/5/examples/ingest/cpp/README.md
File examples/ingest/cpp/README.md:
http://gerrit.cloudera.org:8080/#/c/24201/5/examples/ingest/cpp/README.md@10
PS5, Line 10: Batch inserts with `AUTO_FLUSH_BACKGROUND` session mode
> I'm curious: what amount of performance benefits do you find in using manua
I wanted to benchmark tuning the batch sizes against auto flush, but if the auto_flush is proven to be better in this regard, then there is no point in repeating that measurement. Do you have any docs or results on this?
http://gerrit.cloudera.org:8080/#/c/24201/5/examples/ingest/cpp/kudu_insert.cc
File examples/ingest/cpp/kudu_insert.cc:
http://gerrit.cloudera.org:8080/#/c/24201/5/examples/ingest/cpp/kudu_insert.cc@51
PS5, Line 51:
> Why not to add using directives for STL classes similar to what's done for
Done
http://gerrit.cloudera.org:8080/#/c/24201/5/examples/ingest/cpp/kudu_insert.cc@71
PS5, Line 71:
: struct AppConfig {
> This is confusing: how to provide master's end-point address? Is it from a
Done
http://gerrit.cloudera.org:8080/#/c/24201/5/examples/ingest/cpp/kudu_insert.cc@84
PS5, Line 84: const char* raw_value = getenv(name);
> This looks strange and confusing:
the multiple ways to define the master address is going to be used in later patches, for docker/instrumentation, but I agree it's confusing in this format, I'm updating it to make it a bit more friendly, so removing positional form and documenting explicit precedence: --master > KUDU_MASTER > default.
http://gerrit.cloudera.org:8080/#/c/24201/5/examples/ingest/cpp/kudu_insert.cc@86
PS5, Line 86: return default_value;
: }
: return string(raw_value);
: }
:
: void PrintUsage(const char* binary) {
: cerr << "Usage: " << binary << " <tsv_file_path>"
: << " [--master <host:port>]" << "\n";
: cerr << " tsv_file_path - Path to the TSV file to ingest" << "\n";
: cerr << " --master - Kudu master address (default: localhost:7051"
: << " or $KUDU_MASTER)" << "\n";
: }
:
: bool ParseArgs(const int argc,
: char* argv[],
: AppConfig* config,
: bool& help_requested) {
: help_requested = false;
:
: config->master_address = GetEnvOrDefault(kMasterEnvVar, "localhost:7051");
:
: optind = 1;
: o
> Any chance of using getopt()/getopt_long() or maybe something from the Boos
Done
http://gerrit.cloudera.org:8080/#/c/24201/5/examples/ingest/cpp/kudu_insert.cc@114
PS5, Line 114:
: int option_index = 0;
: int opt = 0;
: while ((opt = getopt_long(argc, argv, "h", kLongOptions, &option_index)) != -1) {
: switch (opt) {
: case 'h':
: help_requested = true;
: return true;
: case 'm':
: config->master_address = optarg;
: break;
: default:
:
> These str-to-int converters are quite lame. They not only accept invalid i
Done
http://gerrit.cloudera.org:8080/#/c/24201/5/examples/ingest/cpp/kudu_insert.cc@129
PS5, Line 129:
> Consider using std::string_view instead if the source string stays in scope
Done
http://gerrit.cloudera.org:8080/#/c/24201/5/examples/ingest/cpp/kudu_insert.cc@269
PS5, Line 269:
> If you call this app "high-performant", you may consider stop using std::en
thanks for pointing this out! done
http://gerrit.cloudera.org:8080/#/c/24201/5/examples/ingest/cpp/kudu_insert.cc@289
PS5, Line 289: KUDU_RETURN_NOT_OK(CreateSchema(&schema));
:
: /
> Why not to use KUDU_RETURN_NOT_OK here and elsewhere instead of this boiler
Done
http://gerrit.cloudera.org:8080/#/c/24201/5/examples/ingest/cpp/kudu_insert.cc@337
PS5, Line 337: 4_t kProgre
> Use std::string_view here instead to useless avoid memory allocation and co
good idea, done
http://gerrit.cloudera.org:8080/#/c/24201/5/examples/ingest/cpp/kudu_insert.cc@348
PS5, Line 348:
> What exceptions do you expect to catch here? Kudu API isn't supposed to th
the std::stoi/stdo::stoll methods can throw out_of_range and invalid_argument exceptions, it was meant to catch those, but it's no longer necessary, since those have been replaced as recommended to the strto64/strto32_adapter methods
http://gerrit.cloudera.org:8080/#/c/24201/5/examples/ingest/cpp/kudu_insert.cc@354
PS5, Line 354: continue;
: }
:
: unique_ptr<KuduInsert> insert(table->NewInsert());
: KuduPartialRow* row = insert->mutable_row();
: Status row_status = PopulateRow(fields, row);
: if (!row_status.ok()) {
: cerr << "Error parsing row " << rows_read << ": "
: << row_status.ToString() << "\n";
: error_count++;
: continue;
: }
:
: Status s = session->Apply(insert.release());
: if (!s.ok()) {
: cerr << "Error applying insert for row " << rows_read << ": "
: << s.ToString() << "\n";
: error_count++;
: continue;
: }
:
: rows_inserted++;
:
: // Progress logging without forcing synchronous flushes.
: if (rows_inserted % kProgressInterval == 0) {
: cout << "Processed " << rows_inserted << " rows...\n";
: }
: }
:
: // Ensure all pending operations are finished before checking errors.
: KUDU_RETURN_NOT_OK(session->Flush());
:
: infile.close();
:
: // Check for errors
: if (session->CountPendingErrors() > 0) {
: vector<KuduError*> errors;
: bool overflow;
: session->GetPendingErrors(&errors, &overflow);
:
: cerr << "\nErrors occurred during insertion:" << "\n";
: int displayed_errors = 0;
: for (const auto* error : errors) {
: if (displayed_errors < 10) { // Only show first 10 errors
: cerr << " - " << error->status().ToString() << "\n";
: displayed_errors++;
: }
: delete error;
: }
:
: if (errors.size() > 10) {
: cerr << " ... and " << (errors.size() - 10) << " more errors" << "\n";
: }
:
: if (overflow) {
: cerr << " - Error buffer overflowed; some errors were discarded" << "\n";
: }
: }
:
: cout << "\n[OK] Successfully processed " << rows_inserted << " rows\n";
: if (error_count > 0) {
: cout << "[ERROR] " << error_count << " rows had errors\n";
: }
: result.rows_read = rows_read;
: result.rows_inserted = rows_inserted;
: result.error_count = error_count;
:
: return Status::OK();
: }
:
: } // namespace
:
: int main(const int argc, char* argv[]) {
: cout << string(80, '=') << '\n';
: cout << "Kudu C++ Client - Data Insertion from TSV\n";
: cout << string(80, '=') << "\n\n";
:
: AppConfig config;
: bool help_requested = false;
: if (!ParseArgs(argc, argv, &config, help_requested)) {
: PrintUsage(argv[0]);
: return 1;
: }
: if (help_requested) {
: PrintUsage(argv[0]);
: return 0;
: }
:
: cout << "TSV File: " << config.file_path << '\n';
: cout << "Connecting to Kudu master at: " << config.master_address << '\n';
:
: // Create Kudu client
: shared_ptr<KuduClient> client;
: Status s = KuduClientBuilder()
: .add_master_server_addr(config.master_address)
: .Build(&client);
:
: if (!s.ok()) {
: cerr << "Error creating Kudu client: " << s.ToString() << "\n";
: return 1;
: }
:
: cout << "[OK] Connected to Kudu master\n\n";
:
: // Create the table
> Separate this ugly blob this into its function, maybe?
yeah, I refactored the whole schema thing into a separate header column_spec.h, so that this file only has the actual logic in it, hopefully that makes this much more readable
http://gerrit.cloudera.org:8080/#/c/24201/5/examples/ingest/cpp/kudu_insert.cc@470
PS5, Line 470: s = InsertDataFromFile(client, config.file_path, ingest_result);
: if (!s.ok()) {
: cerr << "Error inserting data: " << s.ToString() << "\n";
: return 1;
: }
> Why not to use AUTO_FLUSH_BACKGROUND instead? With particular settings for
I wanted to see how much (if any) fine tuning is possible with changing the flush intervals, but I'll switch back to auto_flush_background for now, and revisit that idea later.
--
To view, visit http://gerrit.cloudera.org:8080/24201
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I2d3fb6b031b26c4215de58de2162e060132846b4
Gerrit-Change-Number: 24201
Gerrit-PatchSet: 6
Gerrit-Owner: Zoltan Chovan <zc...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <al...@apache.org>
Gerrit-Reviewer: Attila Bukor <ab...@apache.org>
Gerrit-Reviewer: Gabriella Lotz <lo...@gmail.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Marton Greber <gr...@gmail.com>
Gerrit-Reviewer: Zoltan Chovan <zc...@cloudera.com>
Gerrit-Reviewer: Zoltan Martonka <zm...@cloudera.com>
Gerrit-Comment-Date: Wed, 22 Apr 2026 13:48:06 +0000
Gerrit-HasComments: Yes
[kudu-CR] [ingest] add C++ TSV loader and schema mapping
Posted by "Anonymous Coward (Code Review)" <ge...@cloudera.org>.
Anonymous Coward (763) has posted comments on this change. ( http://gerrit.cloudera.org:8080/24201 )
Change subject: [ingest] add C++ TSV loader and schema mapping
......................................................................
Patch Set 6: Code-Review+1
--
To view, visit http://gerrit.cloudera.org:8080/24201
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I2d3fb6b031b26c4215de58de2162e060132846b4
Gerrit-Change-Number: 24201
Gerrit-PatchSet: 6
Gerrit-Owner: Zoltan Chovan <zc...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <al...@apache.org>
Gerrit-Reviewer: Anonymous Coward (763)
Gerrit-Reviewer: Attila Bukor <ab...@apache.org>
Gerrit-Reviewer: Gabriella Lotz <lo...@gmail.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Marton Greber <gr...@gmail.com>
Gerrit-Reviewer: Zoltan Chovan <zc...@cloudera.com>
Gerrit-Reviewer: Zoltan Martonka <zm...@cloudera.com>
Gerrit-Comment-Date: Wed, 22 Apr 2026 14:35:18 +0000
Gerrit-HasComments: No
[kudu-CR] [ingest] add C++ TSV loader and schema mapping
Posted by "Zoltan Chovan (Code Review)" <ge...@cloudera.org>.
Hello Marton Greber, Zoltan Martonka, Alexey Serbin, Gabriella Lotz, Attila Bukor, Kudu Jenkins,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/24201
to look at the new patch set (#6).
Change subject: [ingest] add C++ TSV loader and schema mapping
......................................................................
[ingest] add C++ TSV loader and schema mapping
Add a standalone C++ ingest client, build configuration, and usage docs
for loading the ClickHouse hits TSV dataset into Kudu. This creates a
high-throughput native baseline that can be benchmarked and tuned as
the foundation for the ingest pipeline work.
Change-Id: I2d3fb6b031b26c4215de58de2162e060132846b4
---
A examples/ingest/cpp/.gitignore
A examples/ingest/cpp/CMakeLists.txt
A examples/ingest/cpp/README.md
A examples/ingest/cpp/column_spec.h
A examples/ingest/cpp/kudu_insert.cc
5 files changed, 1,204 insertions(+), 0 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/01/24201/6
--
To view, visit http://gerrit.cloudera.org:8080/24201
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I2d3fb6b031b26c4215de58de2162e060132846b4
Gerrit-Change-Number: 24201
Gerrit-PatchSet: 6
Gerrit-Owner: Zoltan Chovan <zc...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <al...@apache.org>
Gerrit-Reviewer: Attila Bukor <ab...@apache.org>
Gerrit-Reviewer: Gabriella Lotz <lo...@gmail.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Marton Greber <gr...@gmail.com>
Gerrit-Reviewer: Zoltan Chovan <zc...@cloudera.com>
Gerrit-Reviewer: Zoltan Martonka <zm...@cloudera.com>