bazel: remove numactl #25041

rockwotj · 2025-02-05T15:10:12Z

Incorporate libnuma into the Bazel build. We force dynamic linking because it's LGPL. The dynamic linking forcing is really annoying, but honestly less annoying than the new way of doing things with cc_shared_library. However I ran into issues using the cc_shared_library approach, so I just fallback to using the "old school" stable ways of making shared libraries.

Backports Required

Release Notes

none

dotnwat · 2025-02-07T23:57:00Z

but honestly less annoying than the new way of doing things with cc_shared_library

i'm not saying we should do this, but from what I can tell seastar's libnuma dependency is a single call to mbind in memory.cc to set a policy on some memory regions.

looking at this:

https://github.com/lrita/numa/blob/29f88905963797c57362861a2dd8adb14a6f152a/numa_linux.go#L161

i do wonder if freeing ourselves from the libnuma dependency would be as simple as writing our own version of this

func MBind(addr unsafe.Pointer, length, mode, flags int, nodemask Bitmask) (err error) {
	var mask, maxnode uintptr
	if maxnode = uintptr(nodemask.Len()); maxnode != 0 {
		mask = uintptr(unsafe.Pointer(&nodemask[0]))
	}
	_, _, errno := syscall.Syscall6(syscall.SYS_MBIND, uintptr(addr),
		uintptr(length), uintptr(mode), mask, maxnode, uintptr(flags))
	if errno != 0 {
		err = errno
	}
	return
}

dotnwat · 2025-02-07T23:58:47Z

bazel/thirdparty/Cargo.toml

@@ -6,7 +6,8 @@ edition = "2021"
 publish = false

 [lib]
-path = "/dev/null"
+# Use a fake path here, but rules_rust requires something.
+path = "./dev/null"


nice. i had gone with fake.rs

rockwotj · 2025-02-08T00:03:08Z

but honestly less annoying than the new way of doing things with cc_shared_library

i'm not saying we should do this, but from what I can tell seastar's libnuma dependency is a single call to mbind in memory.cc to set a policy on some memory regions.

looking at this:

https://github.com/lrita/numa/blob/29f88905963797c57362861a2dd8adb14a6f152a/numa_linux.go#L161

i do wonder if freeing ourselves from the libnuma dependency would be as simple as writing our own version of this
func MBind(addr unsafe.Pointer, length, mode, flags int, nodemask Bitmask) (err error) {

	var mask, maxnode uintptr

	if maxnode = uintptr(nodemask.Len()); maxnode != 0 {

		mask = uintptr(unsafe.Pointer(&nodemask[0]))

	}

	_, _, errno := syscall.Syscall6(syscall.SYS_MBIND, uintptr(addr),

		uintptr(length), uintptr(mode), mask, maxnode, uintptr(flags))

	if errno != 0 {

		err = errno

	}

	return

}

ooh maybe I will send a patch to seastar and see what drama unfolds...

rockwotj · 2025-02-17T17:48:38Z

Updated to remove numactl using the pending upstream patch in seastar

rockwotj · 2025-02-17T22:17:00Z

I snuck in a few clang-19 fixes into this PR. I have a lot of the tree building with a couple of small seastar patches.

vbotbuildovich · 2025-02-18T01:47:50Z

CI test results

test results on build#61934

test_id	test_kind	job_url	test_status	passed
rptest.tests.compaction_recovery_test.CompactionRecoveryTest.test_index_recovery	ducktape	https://buildkite.com/redpanda/redpanda/builds/61934#01951640-53cb-4170-b5cc-b598a05ede4e	FLAKY	1/2
rptest.tests.delete_records_test.DeleteRecordsTest.test_delete_records_concurrent_truncations.cloud_storage_enabled=True.truncate_point=start_offset	ducktape	https://buildkite.com/redpanda/redpanda/builds/61934#01951640-53cb-4f0f-acbe-87b2d578f419	FLAKY	1/3
rptest.transactions.producers_api_test.ProducersAdminAPITest.test_producers_state_api_during_load	ducktape	https://buildkite.com/redpanda/redpanda/builds/61934#01951640-53cc-42c1-9675-789a02bdbb75	FLAKY	1/2

test results on build#61942

test_id	test_kind	job_url	test_status	passed
rptest.tests.compaction_recovery_test.CompactionRecoveryTest.test_index_recovery	ducktape	https://buildkite.com/redpanda/redpanda/builds/61942#0195179f-3811-4b73-9a88-cf50fba3c5a4	FLAKY	1/2
rptest.tests.datalake.simple_connect_test.RedpandaConnectIcebergTest.test_translating_avro_serialized_records.cloud_storage_type=CloudStorageType.S3	ducktape	https://buildkite.com/redpanda/redpanda/builds/61942#0195179f-380f-4ec4-acdd-92bc8273a34b	FLAKY	1/2
rptest.tests.log_compaction_test.LogCompactionTest.compaction_stress_test.cleanup_policy=compact.delete.key_set_cardinality=1000.storage_compaction_key_map_memory_kb=10	ducktape	https://buildkite.com/redpanda/redpanda/builds/61942#0195179f-3811-4b73-9a88-cf50fba3c5a4	FLAKY	1/2
rptest.tests.log_compaction_test.LogCompactionTest.compaction_stress_test.cleanup_policy=compact.delete.key_set_cardinality=1000.storage_compaction_key_map_memory_kb=3	ducktape	https://buildkite.com/redpanda/redpanda/builds/61942#0195179f-380f-4ec4-acdd-92bc8273a34b	FLAKY	1/2
rptest.tests.log_compaction_test.LogCompactionTest.compaction_stress_test.cleanup_policy=compact.key_set_cardinality=1000.storage_compaction_key_map_memory_kb=10	ducktape	https://buildkite.com/redpanda/redpanda/builds/61942#0195179f-380f-4ec4-acdd-92bc8273a34b	FLAKY	1/2
rptest.tests.log_compaction_test.LogCompactionTest.compaction_stress_test.cleanup_policy=compact.key_set_cardinality=1000.storage_compaction_key_map_memory_kb=3	ducktape	https://buildkite.com/redpanda/redpanda/builds/61942#0195179f-3811-4b73-9a88-cf50fba3c5a4	FLAKY	1/2
rptest.tests.partition_movement_test.SIPartitionMovementTest.test_shadow_indexing.num_to_upgrade=0.cloud_storage_type=CloudStorageType.ABS	ducktape	https://buildkite.com/redpanda/redpanda/builds/61942#0195179f-3811-4b73-9a88-cf50fba3c5a4	FLAKY	1/2
rptest.tests.random_node_operations_test.RandomNodeOperationsTest.test_node_operations.enable_failures=True.mixed_versions=False.with_tiered_storage=False.with_iceberg=False.with_chunked_compaction=True.cloud_storage_type=CloudStorageType.S3	ducktape	https://buildkite.com/redpanda/redpanda/builds/61942#0195179f-380f-4205-a0ea-b5bd4403ce08	FLAKY	1/2
rptest.tests.random_node_operations_test.RandomNodeOperationsTest.test_node_operations.enable_failures=True.mixed_versions=False.with_tiered_storage=True.with_iceberg=False.with_chunked_compaction=False.cloud_storage_type=CloudStorageType.ABS	ducktape	https://buildkite.com/redpanda/redpanda/builds/61942#0195179f-3811-4b73-9a88-cf50fba3c5a4	FLAKY	1/2
rptest.tests.scaling_up_test.ScalingUpTest.test_scaling_up_with_recovered_topic	ducktape	https://buildkite.com/redpanda/redpanda/builds/61942#0195179f-3811-4b73-9a88-cf50fba3c5a4	FLAKY	1/3
rptest.transactions.consumer_offsets_test.VerifyConsumerOffsetsThruUpgrades.test_consumer_group_offsets.versions_to_upgrade=1	ducktape	https://buildkite.com/redpanda/redpanda/builds/61942#0195179b-9717-4170-a76f-3e7f1a771897	FLAKY	1/2

test results on build#62027

test_id	test_kind	job_url	test_status	passed
rptest.tests.controller_log_limiting_test.ControllerLogLimitMirrorMakerTests.test_mirror_maker_with_limits	ducktape	https://buildkite.com/redpanda/redpanda/builds/62027#01951f49-c215-4f5e-8900-d7dcbffeeef0	FLAKY	1/2
rptest.tests.log_compaction_test.LogCompactionTest.compaction_stress_test.cleanup_policy=compact.delete.key_set_cardinality=1000.storage_compaction_key_map_memory_kb=10	ducktape	https://buildkite.com/redpanda/redpanda/builds/62027#01951f49-c218-4b4a-bd92-52e16dcfb9f1	FLAKY	1/2
rptest.tests.log_compaction_test.LogCompactionTest.compaction_stress_test.cleanup_policy=compact.delete.key_set_cardinality=1000.storage_compaction_key_map_memory_kb=3	ducktape	https://buildkite.com/redpanda/redpanda/builds/62027#01951f49-c215-4f5e-8900-d7dcbffeeef0	FLAKY	1/2

dotnwat · 2025-02-19T04:40:08Z

MODULE.bazel

@@ -47,6 +47,7 @@ bazel_dep(name = "yaml-cpp", version = "0.8.0")
 bazel_dep(name = "zlib", version = "1.3.1.bcr.3")
 bazel_dep(name = "zstd", version = "1.5.6")
 bazel_dep(name = "patchelf", version = "0.18.0")
+bazel_dep(name = "bzip2", version = "1.0.8.bcr.2")


bazel: upgrade bzip2

but this commit is adding bzip2, right? was bzip2 being depended on indirectly, and this new bazel_dep line then causes all bzip2 instances to use the bcr.2 version?

Yeah exactly, you can see this in the lock file

This breaks in bazel 8, so fix it now

required to work with Bazel 8

These have landed upstream

To prevent 500 errors from sourceware.org

Newer libcpp uses a wrapped iterator type for string_view so we aren't comparing the right types between string_view and ss::sstring

In newer libcpp iterators are not pointers, fix the usage here.

To address a clang-19 warning

rockwotj · 2025-02-19T15:51:50Z

Force push: fix conflicts in MODULE.bazel.lock

travisdowns · 2025-02-27T14:02:47Z

@rockwotj is there a specific reason we are applying these changes as patches in our redpanda-side bazel build rather than checking them into our seastar fork? Is it because if we put them into our fork it won't build (cmake style) any more?

rockwotj · 2025-02-27T15:00:27Z

@rockwotj is there a specific reason we are applying these changes as patches in our redpanda-side bazel build rather than checking them into our seastar fork? Is it because if we put them into our fork it won't build (cmake style) any more?

No we should be applying them to our seastar fork, especially since this is upstreamed and not in the cmake build once we rebase.

rockwotj · 2025-02-27T15:01:13Z

See: scylladb/seastar#2645

travisdowns · 2025-02-27T15:08:57Z

See: scylladb/seastar#2645

Very nice. I will cherry pick this into our fork.

rockwotj · 2025-02-27T15:17:25Z

Thanks, it's on my list of things to get too 😅

github-actions bot added the area/build label Feb 5, 2025

rockwotj force-pushed the numactl branch 3 times, most recently from 691a390 to 982e68b Compare February 7, 2025 22:09

rockwotj requested review from dotnwat and mmaslankaprv February 7, 2025 22:10

rockwotj marked this pull request as ready for review February 7, 2025 22:12

dotnwat previously approved these changes Feb 7, 2025

View reviewed changes

rockwotj dismissed dotnwat’s stale review via 4a32715 February 17, 2025 17:46

rockwotj changed the title ~~bazel: fix numactl~~ bazel: remove numactl Feb 17, 2025

rockwotj force-pushed the numactl branch 2 times, most recently from 06d6976 to def94ad Compare February 17, 2025 21:20

github-actions bot added the area/redpanda label Feb 17, 2025

rockwotj requested a review from dotnwat February 17, 2025 21:47

rockwotj requested review from r-vasquez, kbatuigas and a team as code owners February 18, 2025 04:29

github-actions bot added the area/rpk label Feb 18, 2025

dotnwat previously approved these changes Feb 19, 2025

View reviewed changes

rockwotj added 6 commits February 19, 2025 15:49

bazel: correct urls to be a list

e4ad334

This breaks in bazel 8, so fix it now

bazel: upgrade rules rust

ced5449

required to work with Bazel 8

bazel: fix numactl and enforce dynamic linking

99a8ff7

bazel: remove old patches

ea1acd1

These have landed upstream

seastar: remove libnuma dependency

bdfa914

bazel: upgrade bzip2

c291d25

To prevent 500 errors from sourceware.org

rockwotj added 5 commits February 19, 2025 15:51

strings: use raw pointers for compat with newer libcpp

2659e21

Newer libcpp uses a wrapped iterator type for string_view so we aren't comparing the right types between string_view and ss::sstring

iobuf: use string raw pointer

6d81fa6

In newer libcpp iterators are not pointers, fix the usage here.

iobuf: use explicit template name

bf939e9

To address a clang-19 warning

bazel: update rpk BUILD

be86f95

rpk: fix confusing-results lint

0d6b4a4

rockwotj dismissed dotnwat’s stale review via 0d6b4a4 February 19, 2025 15:51

rockwotj force-pushed the numactl branch from 9c4442e to 0d6b4a4 Compare February 19, 2025 15:51

rockwotj enabled auto-merge February 19, 2025 15:51

rockwotj requested a review from dotnwat February 19, 2025 15:51

dotnwat approved these changes Feb 19, 2025

View reviewed changes

rockwotj merged commit d42779f into redpanda-data:dev Feb 19, 2025
24 checks passed

rockwotj deleted the numactl branch February 19, 2025 21:10

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

bazel: remove numactl #25041

bazel: remove numactl #25041

rockwotj commented Feb 5, 2025 •

edited

Loading

dotnwat commented Feb 7, 2025

dotnwat Feb 7, 2025

rockwotj commented Feb 8, 2025

rockwotj commented Feb 17, 2025

rockwotj commented Feb 17, 2025

vbotbuildovich commented Feb 18, 2025 •

edited

Loading

dotnwat Feb 19, 2025

rockwotj Feb 19, 2025

rockwotj commented Feb 19, 2025

travisdowns commented Feb 27, 2025 •

edited

Loading

rockwotj commented Feb 27, 2025

rockwotj commented Feb 27, 2025

travisdowns commented Feb 27, 2025

rockwotj commented Feb 27, 2025

bazel: remove numactl #25041

bazel: remove numactl #25041

Conversation

rockwotj commented Feb 5, 2025 • edited Loading

Backports Required

Release Notes

dotnwat commented Feb 7, 2025

dotnwat Feb 7, 2025

Choose a reason for hiding this comment

rockwotj commented Feb 8, 2025

rockwotj commented Feb 17, 2025

rockwotj commented Feb 17, 2025

vbotbuildovich commented Feb 18, 2025 • edited Loading

CI test results

dotnwat Feb 19, 2025

Choose a reason for hiding this comment

rockwotj Feb 19, 2025

Choose a reason for hiding this comment

rockwotj commented Feb 19, 2025

travisdowns commented Feb 27, 2025 • edited Loading

rockwotj commented Feb 27, 2025

rockwotj commented Feb 27, 2025

travisdowns commented Feb 27, 2025

rockwotj commented Feb 27, 2025

rockwotj commented Feb 5, 2025 •

edited

Loading

vbotbuildovich commented Feb 18, 2025 •

edited

Loading

travisdowns commented Feb 27, 2025 •

edited

Loading