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

Tue Aug 6 14:34:41 JST 2024

oh my..... I had attached wrong one.....

The old one have one compile warning in line at watchdog:3459 ereport(LOG, ...) and it fails also regression tests.

sorry for the inconvenience.

Before re-submit this patch file, I just checked regression tests, compile warning again.

Could you please discard submitted before and consider this as a actual one?

why I have realized so late! sorry again.

---

Regards
Kwangwon Seo

Bitnine Global Inc.
www.bitnine.net

On Monday, August 5th, 2024 at 8:51 PM, Tatsuo Ishii <ishii at postgresql.org> wrote:

> Thank you for the patch! I will look into it.
> 
> Best reagards,
> --
> Tatsuo Ishii
> SRA OSS LLC
> English: http://www.sraoss.co.jp/index_en/
> Japanese:http://www.sraoss.co.jp
> 
> > 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: 28191 bytes
Desc: not available
URL: <http://www.pgpool.net/pipermail/pgpool-hackers/attachments/20240806/001d35fd/attachment-0001.bin>