[pgpool-hackers: 4495] Re: Watchdog and IPv6

[Kwangwon Seo]

Hello it's been a long time.

I just came back with patch files again.

I hope not too late. (sorry actually a little bit late...)

## regression tests

Several tests are conducted with various configuration. hostname is came from /etc/hosts.

1. ipv4 test(127.0.0.1, localhost)
2. ipv6 test(::1, ip6-localhost)
3. combined ipv4 and ipv6 test(node0 : given ipv4, node1 and node2 : given ipv6)

target: 004, 011, 012, 013, 014, 015

## pg_indent

pg_indent is applied to only my changes, not to other existing one.

## p.s.

If you review this patch, I would be happy!

Any kind of suggestion, criticism welcome.

This patch is a proposal, so you can reject this or request some additional things to me.

---

Regards
Kwangwon Seo

Bitnine Global Inc.
www.bitnine.net


On Saturday, July 13th, 2024 at 8:31 PM, Tatsuo Ishii <ishii at postgresql.org> wrote:

> > Hello.
> >
> > thanks for the fast reply with much informative things!!!
> >
> > In short, I'll reimplement about it and submit patch again.
> >
> > here's detail below.
> >
> > - about ipv4-mapped-ipv6 platform support
> > I have found this one
> > https://forums.freebsd.org/threads/creating-a-ipv4-ipv6-socket-in-c.92530/
> > In the thread, there's a link of freebsd source.
> > https://github.com/freebsd/freebsd-src/blob/main/usr.bin/sockstat/sockstat.c#L432
> > So, I think, what I did is not a recommanded way for freeBSD.
> > wow, I didn't know about this. and you're right. I think It's unstable for freeBSD.
> > And I also watch this https://www.pgpool.net/docs/latest/en/html/intro-whatis.html
> > The link says, FreeBSD is obviously one of support architecture of pgpool.
> >
> > - changing implementation direction.
> > Oh I thought that there were some reason for that word what you said (need to fix only 4 function) But It seems not now!
> > Then I can choose pgpool's way, as you recommanded.
> > Yes. It will be a better way for product..!
> >
> > - compiler warning of asprintf() usage
> > Ohh, I have checked multiple times, but I miss that one..
> > I'll handle that warning next time.
> >
> > - replacement of asprintf()
> > I have just checked the psprintf().
> > Yes, I'm agree with your opinion. So I'll replace asprintf() to psprintf() way.
> >
> > - regression tests
> > Oh this is my bad. I just did one test, 004.watchdog in this time.
> > Next time, I'll do regression test with ./regress.sh watchdog*. and also tests all regressions again before submit the patch.
> >
> > - patch files
> > Yes I got it!
> >
> > whoa Thanks for the reviewing my patch. It helps me a lot!!
> >
> > As I mentioned, I want to submit another patch So I'm working on it.
>
>
>
> Thank you for understanding. Looking forward to reviewing your new
> patch.
>
> > If you want to say someting, you can say at ease.
> >
> > Best Regards...
> >
> > Kwangwon Seo
> >
> > Bitnine Global Inc.
> > www.bitnine.net
> >
> > On Saturday, July 13th, 2024 at 07:53, Tatsuo Ishii ishii at postgresql.org wrote:
> >
> > > Hi Kwangwon,
> > >
> > > Thank you for the patch!
> > >
> > > > Oh Hi.. It's been a week...
> > > >
> > > > I think I'm too slow! sorry about that.
> > >
> > > No problem. Doing this work in a week is excellent in my opinion.
> > >
> > > > Anyway, Here's the details and patch files.
> > > >
> > > > - Why chose ipv4-mapped-ipv6 method
> > > > Initially, I want to open multiple sockets for ipv4 socket and ipv6 socket just like pgpool process.
> > > > but If I do so, I also have to modify struct SocketConnection. because it has one socket descriptor.
> > > > That means, the requirements; "we need to fix only 4 functions" is not met.
> > >
> > > Sorry, this is my bad. I did not intend to limit the number of
> > > functions you need to modify. So this is not "requirements".
> > >
> > > > So I chose ipv4-mapped-ipv6 method.
> > >
> > > I am new to ipv4-mapped-ipv6 method. So I am not sure but am afraid
> > > this method is not supported on some plaforms like FreeBSD?
> > >
> > > https://stackoverflow.com/questions/2693709/what-was-the-motivation-for-adding-the-ipv6-v6only-flag
> > >
> > > > Platform support
> > > > Some platforms do not support dualstack sockets, either due to age or deliberate choice.
> > > > For example, FreeBSD does not support IPv4-mapped addresses at all:
> > >
> > > If this is true, better to return to the your original idea?
> > >
> > > > Initially, I want to open multiple sockets for ipv4 socket and ipv6 socket just like pgpool process.
> > >
> > > > - Use of asprintf
> > > > asprintf() is a GNU extension. not a posix, or C standard.
> > > > reference: https://www.gnu.org/software/gnulib/manual/html_node/asprintf.html
> > > > But I think it's still good to use. also, there are some usage of asprintf() in the codes.
> > >
> > > Thanks for pointing it out. Yes, I agree asprintf is useful. But
> > > there's better alternative in pgpool: psprintf(imported from
> > > PostgreSQL). It returns palloced memory (in pgpool backend
> > > process). So you can either explicitly free the memory allocated by
> > > psprintf by using pfree or free the memory altogether in the same
> > > memory context using MemoryContextDelete.
> > >
> > > BTW, I get some compiler warnings regarding asprintf
> > >
> > > watchdog.c: In function ‘wd_create_recv_socket’:
> > > watchdog.c:867:2: warning: ignoring return value of ‘asprintf’, declared with attribute warn_unused_result [-Wunused-result]
> > > 867 | asprintf(&portstr, "%d", port);
> > > | ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> > >
> > > I have a plan to replace all asprintf call to psprintf (PostgreSQL did
> > > that long time ago).
> > >
> > > > - Regression tests
> > > > I tested 004.watchdog with changing hostnames with all branches which is in range of EOL.
> > >
> > > I think you should test at least all watchdog tests. I am getting
> > > timeout error on 028.watchdog_enable_consensus_with_half_votes test.
> > >
> > > > - etc
> > > > I also know that quality is the most important thing.
> > > > So, If I do something wrong, you can reject my patch or request to me to do something..!
> > > >
> > > > - patch files(attachment)
> > > > for V4_1, V4_2, V4_3, V4_4, V4_5, master
> > >
> > > Since this is a new feature, you don't need to create patches other
> > > than master because the patch will be pushed to master branch only.
> > >
> > > > Best Regards...
> > > >
> > > > Kwangwon Seo
> > > >
> > > > Bitnine Global Inc.
> > > > www.bitnine.net
> > > >
> > > > On Thursday, July 4th, 2024 at 16:30, Tatsuo Ishii ishii at postgresql.org wrote:
> > > >
> > > > > > Oh... Can I challenge to that feature implementatation?
> > > > > >
> > > > > > I saw that things ago, todo list and github issue also.
> > > > > >
> > > > > > And I just saw that you updated about that one.
> > > > > >
> > > > > > I was hesitated because I think that is already done or on progress by other developer.
> > > > >
> > > > > As far as I know no other developers are working on this.
> > > > >
> > > > > > If available, can I do this?
> > > > >
> > > > > Sure. Why not. I will volunteer on reviewing your patch.
> > > > >
> > > > > Best reagards,
> > > > > --
> > > > > Tatsuo Ishii
> > > > > SRA OSS LLC
> > > > > English: http://www.sraoss.co.jp/index_en/
> > > > > Japanese:http://www.sraoss.co.jp
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0001-feature-ipv6-for-watchdog.patch
Type: text/x-patch
Size: 28077 bytes
Desc: not available
URL: <http://www.pgpool.net/pipermail/pgpool-hackers/attachments/20240805/c9fc8d7d/attachment-0001.bin>